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

Adding Authorization Code Flow with Proof Key for Code Exchange (PKCE) #151

Closed
wants to merge 1 commit into from

Conversation

martinmake
Copy link

I would start with a discussion about the user interface.
I've added an example for this feature so we can have the same context.

What do you think about it?

Should we use:

  • pub enum AuthorizationFlow to choose between Authorization Flows, or
  • builders/functions, or
  • should it be inferred from secret: Option<String> (won't scale well with other Authorization Flows)?

@marioortizmanero
Copy link
Collaborator

Hi, sorry for not replying earlier. I wanted to read about the PKCE authentication process beforehand. Maybe we should re-think the authentication process? I didn't really like how it turned out in the rewrite because we need stuff like InvalidAuth to for example make sure the me endpoint isn't accessed by a client with the Client Credentials Flow. I was thinking that we could take advantage of Rust's features and only implement these endpoints for some flows and make to make it more type-safe with a different architecture.

I'll open a new issue soon with some ideas where we can discuss how to make it work.

@marioortizmanero
Copy link
Collaborator

This is being discussed at #173 now. Let me know what you think!

@marioortizmanero
Copy link
Collaborator

marioortizmanero commented Mar 9, 2021

Closing this, as #173 is more or less accepted. Thanks for your help, @martinmake.

@marioortizmanero marioortizmanero mentioned this pull request Aug 31, 2021
2 tasks
@marioortizmanero
Copy link
Collaborator

This shouldn't have been closed because PKCE was left as a TODO In that PR. Reopening.

@marioortizmanero
Copy link
Collaborator

Ah sorry! I thought I was in the issue #150 but this was the old PR. Nevermind.

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