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

refactor: pass GrantParams object to issueRefreshToken #662

Merged
merged 1 commit into from
Sep 7, 2022

Conversation

hf
Copy link
Contributor

@hf hf commented Sep 4, 2022

Passes an empty models.GrantParams object to issueRefreshToken as this is going to be used by SAML / MFA.

@hf hf requested a review from a team as a code owner September 4, 2022 21:41
@kangmingtay
Copy link
Member

@hf why would SAML and MFA need this again?

@hf
Copy link
Contributor Author

hf commented Sep 5, 2022

@hf why would SAML and MFA need this again?

So that you can pass parameters for the session such as duration, factor ID, provider ID, etc.

@kangmingtay
Copy link
Member

@hf hmm why are we passing GrantParams into the models/refresh_tokens.go methods (https://github.com/supabase/gotrue/blob/master/models/refresh_token.go#L101)? shouldn't they be tied to the models/sessions.go instead?

@hf
Copy link
Contributor Author

hf commented Sep 6, 2022

@hf hmm why are we passing GrantParams into the models/refresh_tokens.go methods (https://github.com/supabase/gotrue/blob/master/models/refresh_token.go#L101)? shouldn't they be tied to the models/sessions.go instead?

Good point. Let me dig around.

@hf
Copy link
Contributor Author

hf commented Sep 7, 2022

@hf hmm why are we passing GrantParams into the models/refresh_tokens.go methods (https://github.com/supabase/gotrue/blob/master/models/refresh_token.go#L101)? shouldn't they be tied to the models/sessions.go instead?

Good point. Let me dig around.

OK, so today in the code the concept of a session starts on the issuance of the first refresh token. This should be refactored -- but it's going to be a bigger change. Until that happens, we need some way of passing parameters for the session / refresh token, and this is the way today.

@hf hf merged commit 8b1ec24 into master Sep 7, 2022
@hf hf deleted the hf/pass-grant-params-to-issue-refresh-token branch September 7, 2022 08:45
@github-actions
Copy link
Contributor

🎉 This PR is included in version 2.16.5 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

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

Successfully merging this pull request may close these issues.

None yet

3 participants