🌱 Make responseType configurable #143
Conversation
| @@ -48,7 +48,7 @@ const appRoutes: Routes = [ | |||
| }, | |||
| { | |||
| path: 'sessionToken-login', | |||
| component: SessionTokenLogin | |||
| component: SessionTokenLoginComponent | |||
There was a problem hiding this comment.
This looks like a breaking change?
There was a problem hiding this comment.
This is used only in our test suite. We aren't exposing any of this via our SDK, so we aren't introducing a breaking change.
| OktaLoginRedirectComponent | ||
| } from '@okta/okta-angular'; | ||
|
|
||
| describe('Unit Tests', () => { |
There was a problem hiding this comment.
These spec files should be siblings of the source files, it's odd that we're burying unit testing within an e2e test. We should discuss this, we need to decide if we're going to fix this now or later. I also need a place to put unit testing for the user agent work
There was a problem hiding this comment.
As discussed offline: this is going to be significant work and duplication of test boilerplate, so we won't do this.
| @@ -18,6 +18,7 @@ export interface OktaConfig { | |||
| clientId?: string; | |||
| scope?: string; | |||
| onAuthRequired?: Function; | |||
| responseType?: string; | |||
There was a problem hiding this comment.
Should get some README love
| expect(component).toBeTruthy(); | ||
| })); | ||
|
|
||
| it('should instantiate the OktaAuth object', async(() => { |
There was a problem hiding this comment.
We really should be testing that we correctly pass this down to the AuthJS constructor on the login redirect. To do so we would need to write a provider (or service?) wrapper around AuthJS so that it can easily be mocked via DI in test and observed. Out of scope for this PR, since we'll have to touch a lot of code with that change.
| @@ -37,6 +37,7 @@ The `OktaAuthModule` is the initializer for your OpenID Connect client configura | |||
| - `clientId` **(required)**: The OpenID Connect `client_id` | |||
| - `redirectUri` **(required)**: Where the callback is hosted | |||
| - `scope` *(optional)*: Reserved for custom claims to be returned in the tokens | |||
| - `responseType` *(optional)*: Desired token grant types | |||
There was a problem hiding this comment.
I feel like we should be explicitly mentioning that these are string values.
There was a problem hiding this comment.
I thought about noting that in this section, but seems weird to only mention the types on this configuration value. When this value is evaluated, typescript will error due to a type difference.
Uses the new property in AuthJS to append the framework lib to the user agent Also adds unit testing via Jest for React, Vue Angular not yet tested, will add a "unit" test to app.component.spec.ts after #143 lands in master
Uses the new property in AuthJS to append the framework lib to the user agent Also adds unit testing via Jest for React, Vue Angular not yet tested, will add a "unit" test to app.component.spec.ts after #143 lands in master Add lniter to test in vue
Uses the new property in AuthJS to append the framework lib to the user agent Also adds unit testing via Jest for React, Vue Angular not yet tested, will add a "unit" test to app.component.spec.ts after #143 lands in master Add lniter to test in vue
Uses the new property in AuthJS to append the framework lib to the user agent Also adds unit testing via Jest for React, Vue Angular not yet tested, will add a "unit" test to app.component.spec.ts after #143 lands in master Add lniter to test in vue
Uses the new property in AuthJS to append the framework lib to the user agent Also adds unit testing via Jest for React, Vue Angular not yet tested, will add a "unit" test to app.component.spec.ts after #143 lands in master Add lniter to test in vue
Description
Allows the OAuth 2.0
response_typeto be configurable for implicit flows.Important:
codeisn't explicitly supported yet.Resolves: #109