-
Notifications
You must be signed in to change notification settings - Fork 2
feat: syncs time with Authorization Server date header #16
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
6eb2170
fabc10d
c47163b
991f5a5
42b4c83
75f90ad
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -24,7 +24,8 @@ export type OAuth2ClientConfigurations = DiscrimUnion<OAuth2Params & { | |
| export type OAuth2ClientOptions = { | ||
| authentication?: ClientAuthentication; | ||
| allowHTTP?: boolean; | ||
| } | ||
| syncClockWithAuthorizationServer?: boolean; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you anticipate any occasions where a developer wouldn't want to adjust clock skew? In the other SDKs that implement clock skew, the default implementation just does it automatically since it's almost always what the developer would want. If clock skew is not wanted, then it's simpler to assign a different TimeCoordinator instance which doesn't perform clock correction, this way the clock behavior is consistent across the whole SDK. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh, and implementing this here, at the OAuth2 configuration level, doesn't help with clcok skew corrections in other parts of the SDK where they may be necessary.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm aware. I need to address that in a different PR
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. FYI I am solving this problem in #17 |
||
| }; | ||
|
|
||
| /** | ||
| * @group Configuration | ||
|
|
@@ -45,10 +46,25 @@ export class Configuration extends APIClient.Configuration implements APIClientC | |
| * When `true`, issuer and other .well-known endpoints can be HTTP. Defaults to `false` | ||
| */ | ||
| public allowHTTP: boolean = false; | ||
| /** | ||
| * When `true`, the `Date` header from HTTP requests made to the Authorization Server will be | ||
| * used to calculate a clock skew between the Authorization Server and the system clock. This is | ||
| * useful for situations when the client's system clock is set to something other than the "true time". | ||
| * | ||
| * Defaults to `true`. | ||
| * | ||
| * @remarks | ||
| * By default, the `Date` header is not safelisted for CORS requests. The Authorization Server will need | ||
| * to include the `Date` header in the `allow-control-exppose-headers` for this feature to work properly. | ||
| * | ||
| * Reference: https://developer.mozilla.org/en-US/docs/Glossary/CORS-safelisted_response_header | ||
| */ | ||
| public syncClockWithAuthorizationServer: boolean = true; | ||
|
|
||
| public static DefaultOptions: Required<OAuth2ClientOptions> & typeof APIClient.Configuration.DefaultOptions = { | ||
| ...APIClient.Configuration.DefaultOptions, | ||
| allowHTTP: false, | ||
| syncClockWithAuthorizationServer: true, | ||
| authentication: 'none' | ||
| }; | ||
|
|
||
|
|
@@ -61,7 +77,8 @@ export class Configuration extends APIClient.Configuration implements APIClientC | |
| scopes, | ||
| authentication, | ||
| dpop, | ||
| allowHTTP | ||
| allowHTTP, | ||
| syncClockWithAuthorizationServer | ||
| } = { ...Configuration.DefaultOptions, ...params }; | ||
| const url = issuer ?? baseURL; // one of them must be defined via Discriminated Union | ||
| if (!validateURL(url, allowHTTP)) { | ||
|
|
@@ -77,6 +94,7 @@ export class Configuration extends APIClient.Configuration implements APIClientC | |
| // default values are set in `static DefaultOptions` | ||
| this.authentication = authentication; | ||
| this.allowHTTP = allowHTTP; | ||
| this.syncClockWithAuthorizationServer = syncClockWithAuthorizationServer; | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -112,15 +130,16 @@ export class Configuration extends APIClient.Configuration implements APIClientC | |
| } | ||
|
|
||
| toJSON (): JsonRecord { | ||
| const { issuer, discoveryURL, clientId, scopes, authentication, allowHTTP } = this; | ||
| const { issuer, discoveryURL, clientId, scopes, authentication, allowHTTP, syncClockWithAuthorizationServer } = this; | ||
| return { | ||
| ...super.toJSON(), | ||
| issuer: issuer.href, | ||
| discoveryURL: discoveryURL.href, | ||
| clientId, | ||
| scopes, | ||
| authentication, | ||
| allowHTTP | ||
| allowHTTP, | ||
| syncClockWithAuthorizationServer | ||
| }; | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are these Okta-specific errors?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe so, I didn't them mentioned in the spec