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

[BUG] Pyflyte Device Authentication flow does not support PKCE #4735

Open
2 tasks done
EternalDeiwos opened this issue Jan 17, 2024 · 4 comments
Open
2 tasks done

[BUG] Pyflyte Device Authentication flow does not support PKCE #4735

EternalDeiwos opened this issue Jan 17, 2024 · 4 comments
Assignees
Labels
backlogged For internal use. Reserved for contributor team workflow. bug Something isn't working

Comments

@EternalDeiwos
Copy link

Describe the bug

Device Auth fails with a non-descript error message when attempting to authenticate against an IdP that enforces PKCE.

Expected behavior

Pyflyte should use PKCE if it is available with the IdP. At the very least, the error message should indicate the actual problem.

Additional context to reproduce

The IdP must be configured to enforce PKCE on the relevant client. Keycloak will respond with 400 Bad Request if it is expecting a challenge and does not receive one.

Screenshots

No response

Are you sure this issue hasn't been raised already?

  • Yes

Have you read the Code of Conduct?

  • Yes
@EternalDeiwos EternalDeiwos added bug Something isn't working untriaged This issues has not yet been looked at by the Maintainers labels Jan 17, 2024
Copy link

dosubot bot commented Jan 17, 2024

🤖

Hello @EternalDeiwos! I'm Dosu, a friendly bot here to lend a helping hand with your journey through the flyte repository. While we're waiting for a human maintainer, I'm here to help you work through bugs, answer your questions, and guide you on your path to becoming a contributor. Don't hesitate to ask me anything!

Thank you for reporting this issue. Based on the information provided, it appears that Pyflyte does support PKCE (Proof Key for Code Exchange) in its Device Authentication flow. This is evident from the code_challenge_methods_supported field in the OAuth2MetadataResponse message in the Python code here.

As for the error message you're encountering, it would be helpful to know the exact error message you're receiving. In the current implementation, there are several possible error messages related to the authentication process, as seen in the Go test file here. These include "error on callback during authorization due to invalid_request", "could not find the authorize code", "possibly a csrf attack", and "error while exchanging auth code due".

If you could provide the exact error message, it would help in diagnosing the issue further. Also, if you could provide more details about your IdP configuration, especially regarding the enforcement of PKCE, it would be beneficial.

Regarding a similar issue, there was a previous issue titled [Core feature] [flytekit] Device Authflow implementation which was closed without any comments. It's unclear if this issue is directly related to yours, but it might be worth looking into.

I hope this information is helpful. Please provide the additional details requested so we can continue to investigate this issue.

Sources


This response is meant to be useful and save you time. It is not meant to be a precise solution, but rather a starting point for your own research.

Help me be more useful! Please leave a 👍 if this is helpful and 👎 if it is irrelevant.

If you want to continue the conversation, start your reply with @dosu-bot.

@kumare3
Copy link
Contributor

kumare3 commented Jan 17, 2024

Is there suggestion to fallback to using pkce if device code fails?

I do agree error should be better. Can you paste current error?

@EternalDeiwos
Copy link
Author

The current error message is this:

Unable to retrieve Device Authentication Code for {…}, Reason Bad Request

Is there suggestion to fallback to using pkce if device code fails?

This is not about using PKCE as a fallback, this is about enabling PKCE within the OAuth2 Device Auth flow. The current authType=Pkce is a misnomer because it actually refers to OAuth2 Auth Code flow, which can also have PKCE.

@eapolinario eapolinario removed the untriaged This issues has not yet been looked at by the Maintainers label Jan 18, 2024
@eapolinario eapolinario added the backlogged For internal use. Reserved for contributor team workflow. label Jan 18, 2024
@blaketastic2
Copy link

I'm not entirely sure device auth needs to use PKCE as it's a purely back-channel flow. But I guess that's really up to the IDP to enforce, so probably some sort of better error message makes sense.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backlogged For internal use. Reserved for contributor team workflow. bug Something isn't working
Projects
None yet
Development

No branches or pull requests

5 participants