feat(auth): Added code flow support for OIDC flow.#1220
Conversation
hiranya911
left a comment
There was a problem hiding this comment.
Looks good. I mostly had nits. There might be couple of edge cases that need better handling and some tests.
hiranya911
left a comment
There was a problem hiding this comment.
Thanks for making the changes. Looks pretty good. Just one last suggestion to cleanup the large loop in the implementation.
bojeil-google
left a comment
There was a problem hiding this comment.
Change looks good. Just 2 minor issues.
| enabled: true, | ||
| clientId: 'CLIENT_ID', | ||
| issuer: 'https://oidc.com/issuer', | ||
| clientSecret: 'CLIENT_SECRET', |
There was a problem hiding this comment.
I am concerned about legacy customers and would recommend keeping a test without clientSecret and responseType. Can you add new tests instead of modifying the existing ones?
There was a problem hiding this comment.
I add tests for createOAuthIdpConfig and updateOAuthIdpConfig
egilmorez
left a comment
There was a problem hiding this comment.
One optional nit if it's not too big a PITA to regen the reference.
Thanks!
| /** | ||
| * The interface representing OIDC provider's response object for OAuth | ||
| * authorization flow. | ||
| * We need either of them to be true, there are two cases: |
There was a problem hiding this comment.
This comment could be improved. Suggest:
" * One of the following must be true:
- If
codeis set to true, then we are doing code flow. - If
dTokenis set to true, then we are doing ID token flow."
(Assuming that backticks are rendered as code font, and that "ID token flow" is a thing, separate from the literal idToken flag.
RELEASE NOTE: Added code flow support for OIDC flow(previously only support idToken flow).
RELEASE NOTE: Defined
OAuthResponseTypefor specifying responseType either idToken or code.RELEASE NOTE: Defined two new error codes:
INVALID_OAUTH_RESPONSETYPEandMISSING_OAUTH_CLIENT_SECRET.