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

Updates code_challenge documentation #468

Merged
merged 1 commit into from
Mar 26, 2024
Merged

Updates code_challenge documentation #468

merged 1 commit into from
Mar 26, 2024

Conversation

Sgtpluck
Copy link
Contributor

A partner recently struggled a bit with the code_challenge, so I wanted to add a little documentation to make it clearer what is happening.

While doing so, I noticed we had some redundant markdown and the example was incorrect, so I fixed those things.

Base64.encode64(code_challenge) # wrong and URL-unsafe encoding
=> "1BUpxy37SoIPmKw96wbd6MDcvayOYm3ptT+zbe6L/zM=" # wrong and URL-unsafe encoding
# RFC 4648 URL-safe Base64 encoding replaces "+" with "-" and "/" with "_"
=> "1BUpxy37SoIPmKw96wbd6MDcvayOYm3ptT-zbe6L_zM="
Copy link
Contributor Author

Choose a reason for hiding this comment

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

[question] if we want to remove the trailing = we can do that by adding padding: false as an option to Base64.urlsafe_encode64 but it was unclear to me if that was necessary. does anyone know offhand?

Copy link
Contributor

@Jeremy1026 Jeremy1026 Mar 25, 2024

Choose a reason for hiding this comment

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

I'm not sure if we need it or not, but since it works with it (based on my test integration I made forever ago) I'd be inclined to leave it in our docs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it looks like we remove the padding if it exists, so it doesn't matter.

https://github.com/18F/identity-idp/blob/main/app/forms/openid_connect_token_form.rb#L191-L195

@@ -53,13 +52,7 @@ Base64.encode64(code_challenge) # wrong and URL-unsafe encoding
=> "1BUpxy37SoIPmKw96wbd6MDcvayOYm3ptT+zbe6L/zM=" # wrong and URL-unsafe encoding
```
{% endcapture %}
{% capture code_challenge_incorrect %}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this capture is unused

@Sgtpluck Sgtpluck merged commit 2fe27b0 into main Mar 26, 2024
5 checks passed
@Sgtpluck Sgtpluck deleted the dmm/update-code-docs branch March 26, 2024 14:29
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