Skip to content
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

Introduce a dropdown menu for authorization response types #3352

Merged
merged 1 commit into from
Oct 11, 2023

Conversation

Rodien
Copy link
Contributor

@Rodien Rodien commented Oct 3, 2023

Introduce a dropdown menu in the 'External Login' settings area for authentication flow response types.

I used the response types found in the Microsoft.IdentityModel.Protocols.OpenIdConnect.OpenIdConnectResponseType class.

For more information take a look at: #3325

@sbwalker
Copy link
Member

sbwalker commented Oct 4, 2023

@Rodien the majority of these response types are deprecated in OAuth 2.1 because they have security vulnerabilities. This is the reason why Oqtane only included support for Authorization Code Flow (as it is the only recommended approach in OAuth 2.1). So I guess the question is whether or not Oqtane should follow best practices or support legacy implementations. The problem is that if an Oqtane installation gets hacked and its because someone was using a deprecated flow, Oqtane's reputation will still be affected. This is a bit of a moral question - should Oqtane prevent users from exposing themselves to security vulnerabilities?

@Rodien
Copy link
Contributor Author

Rodien commented Oct 4, 2023

@sbwalker You are correct; I was thinking about this today. However, Oqtane should at least support the hybrid flow for now.

The OpenID Connect specification is still based on OAuth 2.0. Therefore, some Identity Providers (IdPs) are still using the 'code id_token' approach.

Another solution would be to make some changes to the token validation inside of Oqtane. Currently, Oqtane expects an email claim. If there is no email claim, Oqtane will throw an error. But what if I want to use something else as a unique identifier?

I mean, Oqtane just needs some unique value to register a user; it doesn't have to be an email address.

Is there a reason why Oqtane specifically expects an email claim?

For example, if I use the Authorization code flow, I will get a token back with the sub claim inside, which has a UUID. Is there a reason why I should not use that value to register a user?

I have been looking around, but I have not found anything yet that says it should not be done that way.

@sbwalker
Copy link
Member

sbwalker commented Oct 4, 2023

Every external user login must be associated to a local user account, as everything in Oqtane is related to a UserId. Oqtane uses .NET Identity and it requires that a user account must have an email address. Therefore an email claim is required for integration.

@Rodien
Copy link
Contributor Author

Rodien commented Oct 6, 2023

Got it

@Rodien
Copy link
Contributor Author

Rodien commented Oct 11, 2023

Okay, so I can make use of the 'OAuth 2.0' provider and the code flow response type. However, I cannot use the OpenID provider type because I need to use the 'codeIdToken' flow, which Oqtane does not support.

So, we can either close this PR or add 'codeIdToken' as a response type in Oqtane. This way, users can use OpenID with the 'codeIdToken' flow (https://auth0.com/blog/backend-for-frontend-pattern-with-auth0-and-dotnet/). In the article above, Okta is using the 'codeIdToken' flow.

However, this PR can be squashed as it is not a showstopper. Users who do encounter this problem can use the OAuth 2.0 provider.

@sbwalker

@sbwalker
Copy link
Member

@Rodien I will merge this enhancement

@sbwalker sbwalker merged commit 6140743 into oqtane:dev Oct 11, 2023
1 check passed
@Rodien
Copy link
Contributor Author

Rodien commented Oct 12, 2023

I would like to note that I only tested the 'Code Id_Token' flow and 'code' flow.

I can make a pull request and remove the rest of the flows, or you can do this yourself if you prefer.

@sbwalker
Copy link
Member

@Rodien I assume you simply included all of the possible response types supported by .NET for OIDC - correct? If so, I think its fine to include them all. Obviously some of them are deprecated or not recommended anymore, but it is possible that some IDP's still use them.

@Rodien
Copy link
Contributor Author

Rodien commented Oct 13, 2023

@sbwalker Yes, correct!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants