mirror of
				https://github.com/optim-enterprises-bv/vault.git
				synced 2025-10-31 02:28:09 +00:00 
			
		
		
		
	OIDC Alternate Path Bug (#17661)
* adds error handling to auth-jwt component for missing roles and fixes bug where role wasn't being retained when using alternate oidc mount path at login * fixes jwt login bug from auth mount tabs and adds test * updates okta-number-challenge success value to arg in template * adds changelog entry * fixes issues logging in manually with jwt * reverts mistaken change
This commit is contained in:
		
							
								
								
									
										3
									
								
								changelog/17661.txt
									
									
									
									
									
										Normal file
									
								
							
							
						
						
									
										3
									
								
								changelog/17661.txt
									
									
									
									
									
										Normal file
									
								
							| @@ -0,0 +1,3 @@ | |||||||
|  | ```release-note:bug | ||||||
|  | ui: Fixes oidc/jwt login issue with alternate mount path and jwt login via mount path tab | ||||||
|  | ``` | ||||||
| @@ -2,7 +2,6 @@ import Ember from 'ember'; | |||||||
| import { next } from '@ember/runloop'; | import { next } from '@ember/runloop'; | ||||||
| import { inject as service } from '@ember/service'; | import { inject as service } from '@ember/service'; | ||||||
| import { match, alias, or } from '@ember/object/computed'; | import { match, alias, or } from '@ember/object/computed'; | ||||||
| import { assign } from '@ember/polyfills'; |  | ||||||
| import { dasherize } from '@ember/string'; | import { dasherize } from '@ember/string'; | ||||||
| import Component from '@ember/component'; | import Component from '@ember/component'; | ||||||
| import { computed } from '@ember/object'; | import { computed } from '@ember/object'; | ||||||
| @@ -284,25 +283,24 @@ export default Component.extend(DEFAULTS, { | |||||||
|   }), |   }), | ||||||
|  |  | ||||||
|   actions: { |   actions: { | ||||||
|     doSubmit(passedData, event) { |     doSubmit(passedData, event, token) { | ||||||
|       if (event) { |       if (event) { | ||||||
|         event.preventDefault(); |         event.preventDefault(); | ||||||
|       } |       } | ||||||
|       let data = {}; |       if (token) { | ||||||
|       this.setProperties({ |         this.set('token', token); | ||||||
|         error: null, |       } | ||||||
|       }); |       this.set('error', null); | ||||||
|       // if callback from oidc we have a token at this point |       // if callback from oidc or jwt we have a token at this point | ||||||
|       let backend = |       const backend = token ? this.getAuthBackend('token') : this.selectedAuthBackend || {}; | ||||||
|         this.providerName === 'oidc' ? this.getAuthBackend('token') : this.selectedAuthBackend || {}; |       const backendMeta = BACKENDS.find( | ||||||
|       let backendMeta = BACKENDS.find( |  | ||||||
|         (b) => (b.type || '').toLowerCase() === (backend.type || '').toLowerCase() |         (b) => (b.type || '').toLowerCase() === (backend.type || '').toLowerCase() | ||||||
|       ); |       ); | ||||||
|       let attributes = (backendMeta || {}).formAttributes || []; |       const attributes = (backendMeta || {}).formAttributes || []; | ||||||
|  |       const data = this.getProperties(...attributes); | ||||||
|  |  | ||||||
|       data = assign(data, this.getProperties(...attributes)); |  | ||||||
|       if (passedData) { |       if (passedData) { | ||||||
|         data = assign(data, passedData); |         Object.assign(data, passedData); | ||||||
|       } |       } | ||||||
|       if (this.customPath || backend.id) { |       if (this.customPath || backend.id) { | ||||||
|         data.path = this.customPath || backend.id; |         data.path = this.customPath || backend.id; | ||||||
|   | |||||||
| @@ -18,6 +18,7 @@ export { ERROR_WINDOW_CLOSED, ERROR_MISSING_PARAMS, ERROR_JWT_LOGIN }; | |||||||
| export default Component.extend({ | export default Component.extend({ | ||||||
|   store: service(), |   store: service(), | ||||||
|   featureFlagService: service('featureFlag'), |   featureFlagService: service('featureFlag'), | ||||||
|  |  | ||||||
|   selectedAuthPath: null, |   selectedAuthPath: null, | ||||||
|   selectedAuthType: null, |   selectedAuthType: null, | ||||||
|   roleName: null, |   roleName: null, | ||||||
| @@ -26,22 +27,18 @@ export default Component.extend({ | |||||||
|   onRoleName() {}, |   onRoleName() {}, | ||||||
|   onLoading() {}, |   onLoading() {}, | ||||||
|   onError() {}, |   onError() {}, | ||||||
|   onToken() {}, |  | ||||||
|   onNamespace() {}, |   onNamespace() {}, | ||||||
|  |  | ||||||
|   didReceiveAttrs() { |   didReceiveAttrs() { | ||||||
|     this._super(); |     this._super(); | ||||||
|     let { oldSelectedAuthPath, selectedAuthPath } = this; |     const debounce = !this.oldSelectedAuthPath && !this.selectedAuthPath; | ||||||
|     let shouldDebounce = !oldSelectedAuthPath && !selectedAuthPath; |  | ||||||
|     if (oldSelectedAuthPath !== selectedAuthPath) { |     if (this.oldSelectedAuthPath !== this.selectedAuthPath || debounce) { | ||||||
|       this.set('role', null); |       this.fetchRole.perform(this.roleName, { debounce }); | ||||||
|       this.onRoleName(this.roleName); |  | ||||||
|       this.fetchRole.perform(null, { debounce: false }); |  | ||||||
|     } else if (shouldDebounce) { |  | ||||||
|       this.fetchRole.perform(this.roleName); |  | ||||||
|     } |     } | ||||||
|  |  | ||||||
|     this.set('errorMessage', null); |     this.set('errorMessage', null); | ||||||
|     this.set('oldSelectedAuthPath', selectedAuthPath); |     this.set('oldSelectedAuthPath', this.selectedAuthPath); | ||||||
|   }, |   }, | ||||||
|  |  | ||||||
|   // Assumes authentication using OIDC until it's known that the mount is |   // Assumes authentication using OIDC until it's known that the mount is | ||||||
| @@ -165,9 +162,7 @@ export default Component.extend({ | |||||||
|     } catch (e) { |     } catch (e) { | ||||||
|       return this.handleOIDCError(e); |       return this.handleOIDCError(e); | ||||||
|     } |     } | ||||||
|     let token = resp.auth.client_token; |     yield this.onSubmit(null, null, resp.auth.client_token); | ||||||
|     this.onToken(token); |  | ||||||
|     yield this.onSubmit(); |  | ||||||
|   }), |   }), | ||||||
|  |  | ||||||
|   actions: { |   actions: { | ||||||
| @@ -177,6 +172,14 @@ export default Component.extend({ | |||||||
|         e.preventDefault(); |         e.preventDefault(); | ||||||
|       } |       } | ||||||
|       if (!this.isOIDC || !this.role || !this.role.authUrl) { |       if (!this.isOIDC || !this.role || !this.role.authUrl) { | ||||||
|  |         let message = this.errorMessage; | ||||||
|  |         if (!this.role) { | ||||||
|  |           message = 'Invalid role. Please try again.'; | ||||||
|  |         } else if (!this.role.authUrl) { | ||||||
|  |           message = | ||||||
|  |             'Missing auth_url. Please check that allowed_redirect_uris for the role include this mount path.'; | ||||||
|  |         } | ||||||
|  |         this.onError(message); | ||||||
|         return; |         return; | ||||||
|       } |       } | ||||||
|       try { |       try { | ||||||
|   | |||||||
| @@ -1,24 +0,0 @@ | |||||||
| import Component from '@glimmer/component'; |  | ||||||
|  |  | ||||||
| /** |  | ||||||
|  * @module OktaNumberChallenge |  | ||||||
|  * OktaNumberChallenge components are used to display loading screen and correct answer for Okta Number Challenge when signing in through Okta |  | ||||||
|  * |  | ||||||
|  * @example |  | ||||||
|  * ```js |  | ||||||
|  * <OktaNumberChallenge @correctAnswer={this.oktaNumberChallengeAnswer} @hasError={this.error} @onReturnToLogin={this.returnToLoginFromOktaNumberChallenge}/> |  | ||||||
|  * ``` |  | ||||||
|  * @param {number} correctAnswer - The correct answer to click for the okta number challenge. |  | ||||||
|  * @param {boolean} hasError - Determines if there is an error being thrown. |  | ||||||
|  * @param {function} onReturnToLogin - Sets waitingForOktaNumberChallenge to false if want to return to main login. |  | ||||||
|  */ |  | ||||||
|  |  | ||||||
| export default class OktaNumberChallenge extends Component { |  | ||||||
|   get oktaNumberChallengeCorrectAnswer() { |  | ||||||
|     return this.args.correctAnswer; |  | ||||||
|   } |  | ||||||
|  |  | ||||||
|   get errorThrown() { |  | ||||||
|     return this.args.hasError; |  | ||||||
|   } |  | ||||||
| } |  | ||||||
| @@ -1,5 +1,5 @@ | |||||||
| <div class="auth-form" data-test-auth-form> | <div class="auth-form" data-test-auth-form> | ||||||
|   {{#if (or (this.error) (and this.waitingForOktaNumberChallenge (not this.cancelAuthForOktaNumberChallenge)))}} |   {{#if (and this.waitingForOktaNumberChallenge (not this.cancelAuthForOktaNumberChallenge))}} | ||||||
|     <OktaNumberChallenge |     <OktaNumberChallenge | ||||||
|       @correctAnswer={{this.oktaNumberChallengeAnswer}} |       @correctAnswer={{this.oktaNumberChallengeAnswer}} | ||||||
|       @hasError={{(not-eq this.error null)}} |       @hasError={{(not-eq this.error null)}} | ||||||
| @@ -74,7 +74,6 @@ | |||||||
|         <AuthJwt |         <AuthJwt | ||||||
|           @onError={{action "handleError"}} |           @onError={{action "handleError"}} | ||||||
|           @onLoading={{action (mut this.isLoading)}} |           @onLoading={{action (mut this.isLoading)}} | ||||||
|           @onToken={{action (mut this.token)}} |  | ||||||
|           @namespace={{this.namespace}} |           @namespace={{this.namespace}} | ||||||
|           @onNamespace={{action (mut this.namespace)}} |           @onNamespace={{action (mut this.namespace)}} | ||||||
|           @onSubmit={{action "doSubmit"}} |           @onSubmit={{action "doSubmit"}} | ||||||
|   | |||||||
| @@ -3,7 +3,7 @@ | |||||||
|     <div class="field has-top-margin-xs"> |     <div class="field has-top-margin-xs"> | ||||||
|       <p data-test-okta-number-challenge-description> |       <p data-test-okta-number-challenge-description> | ||||||
|         To finish signing in, you will need to complete an additional MFA step.</p> |         To finish signing in, you will need to complete an additional MFA step.</p> | ||||||
|       {{#if this.errorThrown}} |       {{#if @hasError}} | ||||||
|         <div class="has-top-margin-s"> |         <div class="has-top-margin-s"> | ||||||
|           <MessageError @errorMessage="There was a problem" /> |           <MessageError @errorMessage="There was a problem" /> | ||||||
|           <button |           <button | ||||||
| @@ -13,7 +13,7 @@ | |||||||
|             data-test-return-from-okta-number-challenge |             data-test-return-from-okta-number-challenge | ||||||
|           >Return to login</button> |           >Return to login</button> | ||||||
|         </div> |         </div> | ||||||
|       {{else if this.oktaNumberChallengeCorrectAnswer}} |       {{else if @correctAnswer}} | ||||||
|         <div class="has-top-margin-s"> |         <div class="has-top-margin-s"> | ||||||
|           <p class="has-text-black has-text-weight-semibold" data-test-okta-number-challenge-verification-type>Okta |           <p class="has-text-black has-text-weight-semibold" data-test-okta-number-challenge-verification-type>Okta | ||||||
|             verification</p> |             verification</p> | ||||||
| @@ -21,7 +21,7 @@ | |||||||
|           <h1 |           <h1 | ||||||
|             class="title has-font-weight-normal has-top-margin-m has-bottom-margin-s" |             class="title has-font-weight-normal has-top-margin-m has-bottom-margin-s" | ||||||
|             data-test-okta-number-challenge-answer |             data-test-okta-number-challenge-answer | ||||||
|           >{{this.oktaNumberChallengeCorrectAnswer}}</h1> |           >{{@correctAnswer}}</h1> | ||||||
|         </div> |         </div> | ||||||
|       {{else}} |       {{else}} | ||||||
|         <div class="has-top-margin-l has-bottom-margin-m"> |         <div class="has-top-margin-l has-bottom-margin-m"> | ||||||
|   | |||||||
| @@ -269,4 +269,49 @@ module('Integration | Component | auth form', function (hooks) { | |||||||
|     ); |     ); | ||||||
|     server.shutdown(); |     server.shutdown(); | ||||||
|   }); |   }); | ||||||
|  |  | ||||||
|  |   test('it should retain oidc role when mount path is changed', async function (assert) { | ||||||
|  |     assert.expect(1); | ||||||
|  |  | ||||||
|  |     const auth_url = 'http://dev-foo-bar.com'; | ||||||
|  |     const server = new Pretender(function () { | ||||||
|  |       this.post('/v1/auth/:path/oidc/auth_url', (req) => { | ||||||
|  |         const { role, redirect_uri } = JSON.parse(req.requestBody); | ||||||
|  |         const goodRequest = | ||||||
|  |           req.params.path === 'foo-oidc' && | ||||||
|  |           role === 'foo' && | ||||||
|  |           redirect_uri.includes('/auth/foo-oidc/oidc/callback'); | ||||||
|  |  | ||||||
|  |         return [ | ||||||
|  |           goodRequest ? 200 : 400, | ||||||
|  |           { 'Content-Type': 'application/json' }, | ||||||
|  |           JSON.stringify( | ||||||
|  |             goodRequest ? { data: { auth_url } } : { errors: [`role "${role}" could not be found`] } | ||||||
|  |           ), | ||||||
|  |         ]; | ||||||
|  |       }); | ||||||
|  |       this.get('/v1/sys/internal/ui/mounts', this.passthrough); | ||||||
|  |     }); | ||||||
|  |  | ||||||
|  |     window.open = (url) => { | ||||||
|  |       assert.strictEqual(url, auth_url, 'auth_url is returned when required params are passed'); | ||||||
|  |     }; | ||||||
|  |  | ||||||
|  |     this.owner.lookup('service:router').reopen({ | ||||||
|  |       urlFor(route, { auth_path }) { | ||||||
|  |         return `/auth/${auth_path}/oidc/callback`; | ||||||
|  |       }, | ||||||
|  |     }); | ||||||
|  |  | ||||||
|  |     this.set('cluster', EmberObject.create({})); | ||||||
|  |     await render(hbs`<AuthForm @cluster={{this.cluster}} />`); | ||||||
|  |  | ||||||
|  |     await component.selectMethod('oidc'); | ||||||
|  |     await component.oidcRole('foo'); | ||||||
|  |     await component.oidcMoreOptions(); | ||||||
|  |     await component.oidcMountPath('foo-oidc'); | ||||||
|  |     await component.login(); | ||||||
|  |  | ||||||
|  |     server.shutdown(); | ||||||
|  |   }); | ||||||
| }); | }); | ||||||
|   | |||||||
| @@ -50,7 +50,6 @@ const renderIt = async (context, path = 'jwt') => { | |||||||
|       @selectedAuthPath={{this.selectedAuthPath}} |       @selectedAuthPath={{this.selectedAuthPath}} | ||||||
|       @onError={{action (mut this.error)}} |       @onError={{action (mut this.error)}} | ||||||
|       @onLoading={{action (mut this.isLoading)}} |       @onLoading={{action (mut this.isLoading)}} | ||||||
|       @onToken={{action (mut this.token)}} |  | ||||||
|       @onNamespace={{action (mut this.namespace)}} |       @onNamespace={{action (mut this.namespace)}} | ||||||
|       @onSelectedAuth={{action (mut this.selectedAuth)}} |       @onSelectedAuth={{action (mut this.selectedAuth)}} | ||||||
|       @onSubmit={{action this.handler}} |       @onSubmit={{action this.handler}} | ||||||
| @@ -73,30 +72,19 @@ module('Integration | Component | auth jwt', function (hooks) { | |||||||
|         return [200, { 'Content-Type': 'application/json' }, JSON.stringify(OIDC_AUTH_RESPONSE)]; |         return [200, { 'Content-Type': 'application/json' }, JSON.stringify(OIDC_AUTH_RESPONSE)]; | ||||||
|       }); |       }); | ||||||
|       this.post('/v1/auth/:path/oidc/auth_url', (request) => { |       this.post('/v1/auth/:path/oidc/auth_url', (request) => { | ||||||
|         let body = JSON.parse(request.requestBody); |         const { role } = JSON.parse(request.requestBody); | ||||||
|         if (body.role === 'test') { |         if (['test', 'okta', 'bar'].includes(role)) { | ||||||
|  |           const auth_url = role === 'test' ? 'http://example.com' : role === 'okta' ? 'http://okta.com' : ''; | ||||||
|           return [ |           return [ | ||||||
|             200, |             200, | ||||||
|             { 'Content-Type': 'application/json' }, |             { 'Content-Type': 'application/json' }, | ||||||
|             JSON.stringify({ |             JSON.stringify({ | ||||||
|               data: { |               data: { auth_url }, | ||||||
|                 auth_url: 'http://example.com', |  | ||||||
|               }, |  | ||||||
|             }), |             }), | ||||||
|           ]; |           ]; | ||||||
|         } |         } | ||||||
|         if (body.role === 'okta') { |         const errors = role === 'foo' ? ['role "foo" could not be found'] : [ERROR_JWT_LOGIN]; | ||||||
|           return [ |         return [400, { 'Content-Type': 'application/json' }, JSON.stringify({ errors })]; | ||||||
|             200, |  | ||||||
|             { 'Content-Type': 'application/json' }, |  | ||||||
|             JSON.stringify({ |  | ||||||
|               data: { |  | ||||||
|                 auth_url: 'http://okta.com', |  | ||||||
|               }, |  | ||||||
|             }), |  | ||||||
|           ]; |  | ||||||
|         } |  | ||||||
|         return [400, { 'Content-Type': 'application/json' }, JSON.stringify({ errors: [ERROR_JWT_LOGIN] })]; |  | ||||||
|       }); |       }); | ||||||
|     }); |     }); | ||||||
|   }); |   }); | ||||||
| @@ -209,8 +197,7 @@ module('Integration | Component | auth jwt', function (hooks) { | |||||||
|     }); |     }); | ||||||
|     this.window.trigger('message', buildMessage()); |     this.window.trigger('message', buildMessage()); | ||||||
|     await settled(); |     await settled(); | ||||||
|     assert.strictEqual(this.token, 'token', 'calls onToken with token'); |     assert.ok(this.handler.withArgs(null, null, 'token').calledOnce, 'calls the onSubmit handler with token'); | ||||||
|     assert.ok(this.handler.calledOnce, 'calls the onSubmit handler'); |  | ||||||
|   }); |   }); | ||||||
|  |  | ||||||
|   test('oidc: fails silently when event origin does not match window origin', async function (assert) { |   test('oidc: fails silently when event origin does not match window origin', async function (assert) { | ||||||
| @@ -240,4 +227,26 @@ module('Integration | Component | auth jwt', function (hooks) { | |||||||
|     await settled(); |     await settled(); | ||||||
|     assert.notOk(this.handler.called, 'should not call the submit handler'); |     assert.notOk(this.handler.called, 'should not call the submit handler'); | ||||||
|   }); |   }); | ||||||
|  |  | ||||||
|  |   test('oidc: it should trigger error callback when role is not found', async function (assert) { | ||||||
|  |     await renderIt(this, 'oidc'); | ||||||
|  |     await component.role('foo'); | ||||||
|  |     await component.login(); | ||||||
|  |     assert.strictEqual( | ||||||
|  |       this.error, | ||||||
|  |       'Invalid role. Please try again.', | ||||||
|  |       'Error message is returned when role is not found' | ||||||
|  |     ); | ||||||
|  |   }); | ||||||
|  |  | ||||||
|  |   test('oidc: it should trigger error callback when role is returned without auth_url', async function (assert) { | ||||||
|  |     await renderIt(this, 'oidc'); | ||||||
|  |     await component.role('bar'); | ||||||
|  |     await component.login(); | ||||||
|  |     assert.strictEqual( | ||||||
|  |       this.error, | ||||||
|  |       'Missing auth_url. Please check that allowed_redirect_uris for the role include this mount path.', | ||||||
|  |       'Error message is returned when role is returned without auth_url' | ||||||
|  |     ); | ||||||
|  |   }); | ||||||
| }); | }); | ||||||
|   | |||||||
| @@ -14,4 +14,7 @@ export default { | |||||||
|   errorMessagePresent: isPresent('[data-test-auth-error]'), |   errorMessagePresent: isPresent('[data-test-auth-error]'), | ||||||
|   descriptionText: text('[data-test-description]'), |   descriptionText: text('[data-test-description]'), | ||||||
|   login: clickable('[data-test-auth-submit]'), |   login: clickable('[data-test-auth-submit]'), | ||||||
|  |   oidcRole: fillable('[data-test-role]'), | ||||||
|  |   oidcMoreOptions: clickable('[data-test-yield-content] button'), | ||||||
|  |   oidcMountPath: fillable('#custom-path'), | ||||||
| }; | }; | ||||||
|   | |||||||
							
								
								
									
										43
									
								
								ui/tests/unit/components/auth-form-test.js
									
									
									
									
									
										Normal file
									
								
							
							
						
						
									
										43
									
								
								ui/tests/unit/components/auth-form-test.js
									
									
									
									
									
										Normal file
									
								
							| @@ -0,0 +1,43 @@ | |||||||
|  | import { module, test } from 'qunit'; | ||||||
|  | import { setupTest } from 'ember-qunit'; | ||||||
|  | import { settled } from '@ember/test-helpers'; | ||||||
|  |  | ||||||
|  | module('Unit | Component | auth-form', function (hooks) { | ||||||
|  |   setupTest(hooks); | ||||||
|  |  | ||||||
|  |   test('it should use token for oidc and jwt auth method types when processing form submit', async function (assert) { | ||||||
|  |     assert.expect(4); | ||||||
|  |  | ||||||
|  |     const component = this.owner.lookup('component:auth-form'); | ||||||
|  |     component.reopen({ | ||||||
|  |       methods: [], // eslint-disable-line | ||||||
|  |       // eslint-disable-next-line | ||||||
|  |       authenticate: { | ||||||
|  |         unlinked() { | ||||||
|  |           return { | ||||||
|  |             perform(type, data) { | ||||||
|  |               assert.deepEqual( | ||||||
|  |                 type, | ||||||
|  |                 'token', | ||||||
|  |                 `Token type correctly passed to authenticate method for ${component.providerName}` | ||||||
|  |               ); | ||||||
|  |               assert.deepEqual( | ||||||
|  |                 data, | ||||||
|  |                 { token: component.token }, | ||||||
|  |                 `Token passed to authenticate method for ${component.providerName}` | ||||||
|  |               ); | ||||||
|  |             }, | ||||||
|  |           }; | ||||||
|  |         }, | ||||||
|  |       }, | ||||||
|  |     }); | ||||||
|  |  | ||||||
|  |     const event = new Event('submit'); | ||||||
|  |  | ||||||
|  |     for (const type of ['oidc', 'jwt']) { | ||||||
|  |       component.set('selectedAuth', type); | ||||||
|  |       await settled(); | ||||||
|  |       await component.actions.doSubmit.apply(component, [undefined, event, 'foo-bar']); | ||||||
|  |     } | ||||||
|  |   }); | ||||||
|  | }); | ||||||
		Reference in New Issue
	
	Block a user
	 Jordan Reimer
					Jordan Reimer