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

Finalizing OAuth 2.0 chapter #1904

Merged
merged 2 commits into from
Mar 17, 2024
Merged

Conversation

csfreak92
Copy link
Collaborator

Ralph made the changes requested by the reviewers based on their comments/suggestions. Also removed the mapping subsection to clean it up.

Ralph made the changes requested by the reviewers based on their comments/suggestions. Also removed the mapping subsection to clean it up.
@csfreak92 csfreak92 requested a review from tghosth March 17, 2024 15:23
Copy link
Member

@jmanico jmanico left a comment

Choose a reason for hiding this comment

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

There are a few grammar changes I want to suggest, but I think we should commit this and charge forward and I'll suggest those changes separately

@@ -28,16 +28,10 @@ There are various different personas in the OAuth process, described in more det

| # | Description | L1 | L2 | L3 |
| :---: | :--- | :---: | :---: | :---: |
| **51.3.1** | [ADDED] Verify that Resource Servers are using mechanisms for sender-constraining access tokens to prevent token replay, such as Mutual TLS for OAuth 2.0 or OAuth Demonstration of Proof of Possession (DPoP). | ✓ | ✓ | ✓ |
| **51.3.1** | [ADDED] Verify Resource Servers implement sender-constrained access token mechanisms to mitigate token replay risks. This is achieved by mandating Mutual TLS for OAuth 2.0 or OAuth Demonstration of Proof of Possession (DPoP), using the client secret provided at client registration. | ✓ | ✓ | ✓ |

Choose a reason for hiding this comment

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

"using the client secret provided"

I think that the use of "client secret" as a client authentication mechanism may pose security risks. Maybe we should consider recommending a secure client authentication mechanism?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think is is in response to Jim's point here:
#1804 (comment)

I am going to merge this for now @VinodAnandan but please do feel free to open a separate issue if you think this is still unclear.

@@ -28,16 +28,10 @@ There are various different personas in the OAuth process, described in more det

| # | Description | L1 | L2 | L3 |
| :---: | :--- | :---: | :---: | :---: |
| **51.3.1** | [ADDED] Verify that Resource Servers are using mechanisms for sender-constraining access tokens to prevent token replay, such as Mutual TLS for OAuth 2.0 or OAuth Demonstration of Proof of Possession (DPoP). | ✓ | ✓ | ✓ |
| **51.3.1** | [ADDED] Verify Resource Servers implement sender-constrained access token mechanisms to mitigate token replay risks. This is achieved by mandating Mutual TLS for OAuth 2.0 or OAuth Demonstration of Proof of Possession (DPoP), using the client secret provided at client registration. | ✓ | ✓ | ✓ |
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think is is in response to Jim's point here:
#1804 (comment)

I am going to merge this for now @VinodAnandan but please do feel free to open a separate issue if you think this is still unclear.

@tghosth tghosth merged commit a57dc21 into OWASP:master-v51-oauth Mar 17, 2024
2 checks passed
tghosth added a commit that referenced this pull request Mar 17, 2024
… (discussed in #996)

* Adding OAuth requirements draft v1

Adding OAuth requirements draft v1

* Cleaning up trimmed OAuth 2.0 requirements

Need to add a few more info and references.

* Final clean-up and updating of OAuth terminology

Final clean-up and updating of OAuth terminology, plus added RFC references.

* Trimming down new OAuth 2.0 requirements

Trimming down new OAuth 2.0 requirements by removing the Authorization Server pieces.

* Minor tweaks to OAuth wording.

* Update 0x12-V3-Session-management.md

Latest OAuth 2.0 requirements from RFC v24

* Move OAuth from @csfreak92 content to a dedicated chapter

* Fix some space issues

* Tidying and formatting and numbering changes

* More linting fixes

* Linting fixes

* More linting fixes

* Fix chapter levels

* category title level and number fix

* Other subtle wording changes

* Update 0x51-V51-OAuth2 chapter (#1880)

* Update 0x51-V51-OAuth2.md

Final edits for OAuth 2.0 protocol chapter

* Update 0x51-V51-OAuth2.md

Cleaning up trailing spaces

* Fixing lint errors

Changed tabs to spaces to show indentation on the terminology section.

* Various tweaks throughout the document

---------

Co-authored-by: Josh Grossman <tghosth@users.noreply.github.com>

* Finalizing OAuth 2.0 chapter (#1904)

* Finalizing OAuth 2.0 chapter

Ralph made the changes requested by the reviewers based on their comments/suggestions. Also removed the mapping subsection to clean it up.

* Update 0x51-V51-OAuth2.md

---------

Co-authored-by: Josh Grossman <tghosth@users.noreply.github.com>

---------

Co-authored-by: Ralph Andalis <ralph.andalis92@gmail.com>
Co-authored-by: Elar Lang <47597707+elarlang@users.noreply.github.com>
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.

4 participants