mirror of
				https://github.com/optim-enterprises-bv/vault.git
				synced 2025-10-31 02:28:09 +00:00 
			
		
		
		
	OIDC Auth Bug (#13133)
* fixes issue with oidc auth method when MetaMask chrome extenstion is used * adds changelog entry * updates auth-jwt integration tests * fixes race condition in runCommands ui-panel helper method where running multiple commands would not always result in the same output order
This commit is contained in:
		
							
								
								
									
										3
									
								
								changelog/13133.txt
									
									
									
									
									
										Normal file
									
								
							
							
						
						
									
										3
									
								
								changelog/13133.txt
									
									
									
									
									
										Normal file
									
								
							| @@ -0,0 +1,3 @@ | |||||||
|  | ```release-note:bug | ||||||
|  | ui: Fixes issue with OIDC auth workflow when using MetaMask Chrome extension | ||||||
|  | ``` | ||||||
| @@ -89,12 +89,18 @@ export default Component.extend({ | |||||||
|     // start watching the popup window and the current one |     // start watching the popup window and the current one | ||||||
|     this.watchPopup.perform(oidcWindow); |     this.watchPopup.perform(oidcWindow); | ||||||
|     this.watchCurrent.perform(oidcWindow); |     this.watchCurrent.perform(oidcWindow); | ||||||
|     // wait for message posted from popup |     // wait for message posted from oidc callback | ||||||
|  |     // see issue https://github.com/hashicorp/vault/issues/12436 | ||||||
|  |     // ensure that postMessage event is from expected source | ||||||
|  |     while (true) { | ||||||
|       const event = yield waitForEvent(thisWindow, 'message'); |       const event = yield waitForEvent(thisWindow, 'message'); | ||||||
|     if (event.origin === thisWindow.origin && event.isTrusted) { |       if (event.origin !== thisWindow.origin || !event.isTrusted) { | ||||||
|       this.exchangeOIDC.perform(event.data, oidcWindow); |         return this.handleOIDCError(); | ||||||
|     } else { |       } | ||||||
|       this.handleOIDCError(); |       if (event.data.source === 'oidc-callback') { | ||||||
|  |         return this.exchangeOIDC.perform(event.data, oidcWindow); | ||||||
|  |       } | ||||||
|  |       // continue to wait for the correct message | ||||||
|     } |     } | ||||||
|   }), |   }), | ||||||
|  |  | ||||||
|   | |||||||
| @@ -9,7 +9,8 @@ export default Route.extend({ | |||||||
|     let { auth_path: path, code, state } = this.paramsFor(this.routeName); |     let { auth_path: path, code, state } = this.paramsFor(this.routeName); | ||||||
|     let { namespaceQueryParam: namespace } = this.paramsFor('vault.cluster'); |     let { namespaceQueryParam: namespace } = this.paramsFor('vault.cluster'); | ||||||
|     path = window.decodeURIComponent(path); |     path = window.decodeURIComponent(path); | ||||||
|     let queryParams = { namespace, path, code, state }; |     const source = 'oidc-callback'; // required by event listener in auth-jwt component | ||||||
|  |     let queryParams = { source, namespace, path, code, state }; | ||||||
|     window.opener.postMessage(queryParams, window.origin); |     window.opener.postMessage(queryParams, window.origin); | ||||||
|   }, |   }, | ||||||
|   renderTemplate() { |   renderTemplate() { | ||||||
|   | |||||||
| @@ -211,7 +211,10 @@ module('Integration | Component | auth jwt', function(hooks) { | |||||||
|     await waitUntil(() => { |     await waitUntil(() => { | ||||||
|       return this.openSpy.calledOnce; |       return this.openSpy.calledOnce; | ||||||
|     }); |     }); | ||||||
|     this.window.trigger('message', buildMessage({ data: { state: 'state', foo: 'bar' } })); |     this.window.trigger( | ||||||
|  |       'message', | ||||||
|  |       buildMessage({ data: { source: 'oidc-callback', state: 'state', foo: 'bar' } }) | ||||||
|  |     ); | ||||||
|     run.cancelTimers(); |     run.cancelTimers(); | ||||||
|     assert.equal(this.error, ERROR_MISSING_PARAMS, 'calls onError with params missing error'); |     assert.equal(this.error, ERROR_MISSING_PARAMS, 'calls onError with params missing error'); | ||||||
|   }); |   }); | ||||||
| @@ -228,6 +231,7 @@ module('Integration | Component | auth jwt', function(hooks) { | |||||||
|       'message', |       'message', | ||||||
|       buildMessage({ |       buildMessage({ | ||||||
|         data: { |         data: { | ||||||
|  |           source: 'oidc-callback', | ||||||
|           path: 'foo', |           path: 'foo', | ||||||
|           state: 'state', |           state: 'state', | ||||||
|           code: 'code', |           code: 'code', | ||||||
| @@ -253,6 +257,7 @@ module('Integration | Component | auth jwt', function(hooks) { | |||||||
|       buildMessage({ |       buildMessage({ | ||||||
|         origin: 'http://hackerz.com', |         origin: 'http://hackerz.com', | ||||||
|         data: { |         data: { | ||||||
|  |           source: 'oidc-callback', | ||||||
|           path: 'foo', |           path: 'foo', | ||||||
|           state: 'state', |           state: 'state', | ||||||
|           code: 'code', |           code: 'code', | ||||||
| @@ -277,6 +282,7 @@ module('Integration | Component | auth jwt', function(hooks) { | |||||||
|       buildMessage({ |       buildMessage({ | ||||||
|         isTrusted: false, |         isTrusted: false, | ||||||
|         data: { |         data: { | ||||||
|  |           source: 'oidc-callback', | ||||||
|           path: 'foo', |           path: 'foo', | ||||||
|           state: 'state', |           state: 'state', | ||||||
|           code: 'code', |           code: 'code', | ||||||
|   | |||||||
| @@ -1,5 +1,6 @@ | |||||||
| import { text, triggerable, clickable, collection, fillable, value, isPresent } from 'ember-cli-page-object'; | import { text, triggerable, clickable, collection, fillable, value, isPresent } from 'ember-cli-page-object'; | ||||||
| import { getter } from 'ember-cli-page-object/macros'; | import { getter } from 'ember-cli-page-object/macros'; | ||||||
|  | import { settled } from '@ember/test-helpers'; | ||||||
|  |  | ||||||
| import keys from 'vault/lib/keycodes'; | import keys from 'vault/lib/keycodes'; | ||||||
|  |  | ||||||
| @@ -45,6 +46,7 @@ export default { | |||||||
|     for (let command of toExecute) { |     for (let command of toExecute) { | ||||||
|       await this.consoleInput(command); |       await this.consoleInput(command); | ||||||
|       await this.enter(); |       await this.enter(); | ||||||
|  |       await settled(); | ||||||
|     } |     } | ||||||
|   }, |   }, | ||||||
| }; | }; | ||||||
|   | |||||||
							
								
								
									
										65
									
								
								ui/tests/unit/components/auth-jwt-test.js
									
									
									
									
									
										Normal file
									
								
							
							
						
						
									
										65
									
								
								ui/tests/unit/components/auth-jwt-test.js
									
									
									
									
									
										Normal file
									
								
							| @@ -0,0 +1,65 @@ | |||||||
|  | import { module, test } from 'qunit'; | ||||||
|  | import { setupTest } from 'ember-qunit'; | ||||||
|  | import EmberObject from '@ember/object'; | ||||||
|  | import Evented from '@ember/object/evented'; | ||||||
|  | import sinon from 'sinon'; | ||||||
|  | import { run } from '@ember/runloop'; | ||||||
|  |  | ||||||
|  | const mockWindow = EmberObject.extend(Evented, { | ||||||
|  |   origin: 'http://localhost:4200', | ||||||
|  | }); | ||||||
|  |  | ||||||
|  | module('Unit | Component | auth-jwt', function(hooks) { | ||||||
|  |   setupTest(hooks); | ||||||
|  |  | ||||||
|  |   hooks.beforeEach(function() { | ||||||
|  |     this.component = this.owner.lookup('component:auth-jwt'); | ||||||
|  |     this.component.set('window', mockWindow.create()); | ||||||
|  |     this.errorSpy = sinon.spy(this.component, 'handleOIDCError'); | ||||||
|  |   }); | ||||||
|  |  | ||||||
|  |   test('it should handle error for cross origin messages while waiting for oidc callback', async function(assert) { | ||||||
|  |     assert.expect(1); | ||||||
|  |     this.component.prepareForOIDC.perform(mockWindow.create()); | ||||||
|  |     this.component.window.trigger('message', { origin: 'http://anotherdomain.com', isTrusted: true }); | ||||||
|  |     assert.ok(this.errorSpy.calledOnce, 'Error handled from cross origin window message event'); | ||||||
|  |     run.cancelTimers(); | ||||||
|  |   }); | ||||||
|  |  | ||||||
|  |   test('it should handle error for untrusted messages while waiting for oidc callback', async function(assert) { | ||||||
|  |     assert.expect(1); | ||||||
|  |     this.component.prepareForOIDC.perform(mockWindow.create()); | ||||||
|  |     this.component.window.trigger('message', { origin: 'http://localhost:4200', isTrusted: false }); | ||||||
|  |     assert.ok(this.errorSpy.calledOnce, 'Error handled from untrusted window message event'); | ||||||
|  |     run.cancelTimers(); | ||||||
|  |   }); | ||||||
|  |   // test case for https://github.com/hashicorp/vault/issues/12436 | ||||||
|  |   test('it should ignore messages sent from outside the app while waiting for oidc callback', async function(assert) { | ||||||
|  |     assert.expect(2); | ||||||
|  |     this.component.prepareForOIDC.perform(mockWindow.create()); | ||||||
|  |     const message = { | ||||||
|  |       origin: 'http://localhost:4200', | ||||||
|  |       isTrusted: true, | ||||||
|  |       data: { | ||||||
|  |         namespace: 'foobar', | ||||||
|  |         path: '/foo/bar', | ||||||
|  |         state: 'authorized', | ||||||
|  |         code: 204, | ||||||
|  |       }, | ||||||
|  |     }; | ||||||
|  |  | ||||||
|  |     this.component.window.trigger('message', message); | ||||||
|  |     message.data.source = 'foo-bar'; | ||||||
|  |     this.component.window.trigger('message', message); | ||||||
|  |     message.data.source = 'oidc-callback'; | ||||||
|  |     this.component.window.trigger('message', message); | ||||||
|  |  | ||||||
|  |     assert.ok(this.errorSpy.notCalled, 'Error handler not triggered while waiting for oidc callback message'); | ||||||
|  |     assert.equal( | ||||||
|  |       this.component.exchangeOIDC.performCount, | ||||||
|  |       1, | ||||||
|  |       'exchangeOIDC method fires when oidc callback message is received' | ||||||
|  |     ); | ||||||
|  |     run.cancelTimers(); | ||||||
|  |   }); | ||||||
|  | }); | ||||||
		Reference in New Issue
	
	Block a user
	 Jordan Reimer
					Jordan Reimer