Add CSRF protection by generating/verifying state#2
Merged
radiantshaw merged 4 commits intodevelopfrom Jan 19, 2026
Merged
Conversation
Created the `::Vauth::AuthorizationRequest` class which currently has the ability to construct the Authorization URL via the `#url` method. It needs the `::Vauth::Client` to construct the URL. Also had to modify the `::Vauth::Client` struct to now hold the `:authorization_uri` value. Also fixed some RuboCop issues in other places, and modified some default RuboCop rules.
Added the ability to generate the state of a request to the `::Vauth::AuthorizationRequest` class. When a state is not passed, it represents a new request to be made by the Resource Owner. When a state is passed, it represents a previously made request by the Resource Owner. The ability to verify is given to the `::Vauth::AuthCodeGrant` object. It needs the request instance, and the received state from the redirect. The state value returned by the `::Vauth::AuthorizationRequest#state` method should be stored in a secured storage (like Rails sessions) before redirecting the Resource Owner to the Authorization URI. If the stored and received states don't match, then the `::Vauth::AuthCodeGrant` instance is not available and an error is thrown. The thought behind this is that if there's a state mismatch, then you're not given the grant to fetch the ID Token (and Access Token, but it's not implemented). Also modified a RuboCop rule for the block length metric. I'm just gonna keep increasing the number for now to see how big the test blocks get. I might just disable this metric for tests, but at the same time I don't want the tests to get out of hand. Let's see!
`token_uri` was wrapped, but `authorization_uri` wasn't. Also, I'm not sure if everytime `::Vauth::Client#token_uri` or `::Vauth::Client#authorization_uri` is called, it should return a new instance of `URI`. I feel like it should be the responsibility of the consumers of this class to do the duplication if they need it. But, I'm not stressing about it right now, so we should be good!
Renamed it to `::Vauth::AuthorizationCodeGrant` for no particular reason other than feeling like "Auth" should be "Authorization".
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds CSRF protection via the
stateURL parameter.::Vauth::AuthenticationRequestgenerates a random state for each instance. This should be accessed via the#statemethod and stored in a secured place (like Rails session). After the redirect, this stored state should be given back to this class to construct the "same" request again. This should then be passed to the::Vauth::AuthorizationCodeGrantclass along with the received state to verify it.Notice that
::Vauth::AuthCodeGrantis now called::Vauth::AuthorizationCodeGrant.Changed some RuboCop rules as well.