Skip to content

Conversation

@ScruffyProdigy
Copy link
Contributor

@ScruffyProdigy ScruffyProdigy commented May 3, 2021

Provides an option for developers to specify the OAuth response type for their OIDC provider (either one of these can be set:):

  • id_token
  • code (if set, must also set the client secret)

RELEASE NOTE: Added support for configuring the authorization code flow for OIDC providers.

@ScruffyProdigy
Copy link
Contributor Author

@lsirac

@ScruffyProdigy ScruffyProdigy changed the title Code flow fix(auth): Enable OIDC auth code flow May 3, 2021
@lsirac lsirac changed the title fix(auth): Enable OIDC auth code flow feat(auth): enables OIDC auth code flow Jun 2, 2021
@lsirac lsirac requested a review from hiranya911 June 2, 2021 21:16
Copy link
Contributor

@hiranya911 hiranya911 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks mostly good. Needs a few tweaks to the public API, and a couple of readability nits.

Copy link
Contributor

@hiranya911 hiranya911 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks mostly good. Please fix the errors reported by the linter. Plus couple of minor nits on style.

Copy link
Contributor

@hiranya911 hiranya911 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There seem to be some redundant code mistakenly included in the PR. PTAL.

req = recorder[0]
assert req.method == 'PATCH'
mask = ['clientId', 'displayName', 'enabled', 'issuer']
mask = ['clientId', 'clientSecret', 'displayName', 'enabled', 'issuer', 'responseType.code', 'responseType.idToken']
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The following lines also seem to be triggering more lint errors.

Running linter on module tests
************* Module tests.test_auth_providers
tests/test_auth_providers.py:124:0: C0301: Line too long (111/100) (line-too-long)
tests/test_auth_providers.py:208:0: C0301: Line too long (111/100) (line-too-long)
tests/test_auth_providers.py:228:0: C0301: Line too long (124/100) (line-too-long)
tests/test_auth_providers.py:253:0: C0301: Line too long (127/100) (line-too-long)

@lahirumaramba
Copy link
Member

Thanks @ScruffyProdigy !
Hi @egilmorez please take a look at the reference docs when you get a chance. Thank you!

Copy link
Contributor

@hiranya911 hiranya911 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. LGTM 👍

@lahirumaramba lahirumaramba removed the request for review from egilmorez December 9, 2021 22:50
(optional). A user cannot sign in using a disabled provider.
client_secret: A string which sets the client secret for the new provider.
This is required for the code flow.
code_response_type: Sets whether to enable the code response flow for the new provider.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this a boolean?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. Fixed

Copy link

@egilmorez egilmorez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@lahirumaramba lahirumaramba merged commit 008b1d8 into firebase:master Dec 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants