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

proposal: add/merge OIDC requirements into OAuth2 paragraph (instead of separate OIDC paragraph) #2039

Open
elarlang opened this issue Aug 31, 2024 · 9 comments
Assignees
Labels
1) Discussion ongoing Issue is opened and assigned but no clear proposal yet V51 Group issues related to OAuth _5.0 - prep This needs to be addressed to prepare 5.0

Comments

@elarlang
Copy link
Collaborator

elarlang commented Aug 31, 2024

I prefer to have OAuth2 and OIDC requirement in one main paragraph - those are implemented hand-in-hand and it could be confusing to search them from separate paragraphs from the ASVS

This is just an initial idea, if it does not work out, we can split them to separate paragraphs.

So, any arguments against to put OAuth and OIDC to one paragraph?

(will be implemented, if there is no feedback in one week)

@elarlang elarlang added the V51 Group issues related to OAuth label Aug 31, 2024
@jmanico
Copy link
Member

jmanico commented Aug 31, 2024

OAuth2 and OIDC are used for such different purposes, my instinct it to keep them separate.

@elarlang
Copy link
Collaborator Author

Just in case, explanation for the options.

My proposed option.

# V51 OAuth and OIDC

## V51.1 OAuth generic

## V51.2 OAuth Authorization Server

...

## V51.5 OIDC generic

## V51.6 OIDC authorization server

Vs alternative:

# V51 OAuth

## V51.1 OAuth generic

## V51.2 OAuth Authorization Server


# V52 OIDC

## V52.1 OIDC generic

## V52.2 OIDC authorization server

@jmanico
Copy link
Member

jmanico commented Aug 31, 2024

I like your proposed option!

@TobiasAhnoff
Copy link

TobiasAhnoff commented Sep 1, 2024

I also like the proposed option from @elarlang, since RFCs , BCPs and security profiles for OAuth/OIDC overlap more and more.
For ASVS I think it is important that, if using just "pure" OAuth grants (no id tokens are returned), it should be easy to only apply OAuth verifications (e g when using client-credentials). And I think the proposed option supports that in a good way, given that OAuth verifications are kept clean from OIDC stuff and that the scope for both OAuth and OIDC is well defined.

Note that if the chapter is built this way then 51.1.1 and 51.1.2 needs to be removed/modified so all OIDC details are separated from OAuth verfications

@elarlang
Copy link
Collaborator Author

elarlang commented Sep 2, 2024

My proposed structure leads to another question, how to put introtext info to the document. At the moment @TobiasAhnoff has proposed 2 separate texts, one for OAuth and another for OIDC.

My first instinct and idea is to combine them to one intro chapter texts with the clear goal to point out their differences - that those are used together, but the point and the goal for them is different.

@tghosth tghosth added 1) Discussion ongoing Issue is opened and assigned but no clear proposal yet _5.0 - prep This needs to be addressed to prepare 5.0 labels Sep 2, 2024
@csfreak92
Copy link
Collaborator

I like this proposed structure, @elarlang. Normally, I would say v51 for OAuth 2.0 and v52 for OpenID Connect, but since they really go hand-in-hand and to @TobiasAhnoff's point, it covers the case of "pure" OAuth grants in that structure.

For the proposed texts, I can help separating or even combining them. This is the reason I like the v51 and v52 (alternate structure) because there is clear separation of texts from OAuth and OIDC. If I will be the one to decide, I would do the alternate structure as things are separate and clear, but I think we can try an attempt to clarify things with the proposed structure too. It's worth a try.

@randomstuff
Copy link

randomstuff commented Sep 5, 2024

This is the reason I like the v51 and v52 (alternate structure) because there is clear separation of texts from OAuth and OIDC.

I would think that having clearly separating chapters (if well done) would prevent people from mixing the two topics up. I feel that people often do not see the difference between OAuth and OIDC which leads to architectural/implementation issues.

However, the overlap between the two chapters might be difficult to handle.

@TobiasAhnoff
Copy link

One reason for having one chapter is that when using OIDC you get the full picture in one chapter and less overlap and duplicated text to maintain, but with the risk of not having a clear separation...

Maybe we just have to pick one and try? It is easy to copy paste between the two anyway since OAuth is separate from OIDC in both alternatives. The only thing that needs to be rewritten is the first sections of scope text.

Hard to decide...if I have to choose I think I go for trying with one OAuth2 and OIDC chapter (as I believe the separation will be good enough)

@elarlang
Copy link
Collaborator Author

@TobiasAhnoff - let's go for it - let's put my proposed structure (#2039 (comment)) and your proposed texts (#2039 (comment)) and get things moving. We going to make changes, fixes and update to it countless time in the future, but we need to get moving.

elarlang pushed a commit to elarlang/ASVS that referenced this issue Sep 17, 2024
tghosth pushed a commit that referenced this issue Sep 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1) Discussion ongoing Issue is opened and assigned but no clear proposal yet V51 Group issues related to OAuth _5.0 - prep This needs to be addressed to prepare 5.0
Projects
None yet
Development

No branches or pull requests

6 participants