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

Restructure auth #1057

Merged
merged 4 commits into from
Sep 17, 2024
Merged

Restructure auth #1057

merged 4 commits into from
Sep 17, 2024

Conversation

mdellweg
Copy link
Member

@mdellweg mdellweg commented Sep 5, 2024

No description provided.

@mdellweg mdellweg marked this pull request as ready for review September 5, 2024 11:25
@mdellweg mdellweg enabled auto-merge (rebase) September 5, 2024 11:25
@mdellweg mdellweg force-pushed the auth_power branch 2 times, most recently from af2b3c6 to d006dad Compare September 10, 2024 12:50
Copy link
Contributor

@ggainey ggainey left a comment

Choose a reason for hiding this comment

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

One minor change requested for the public changelog, please

@@ -0,0 +1 @@
Removed the prompt for a username.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should explicitly call out "you'll need to specify a BasicAuth username in the config now", since this is a pretty visible user-facing change.

Copy link
Member Author

Choose a reason for hiding this comment

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

"--username" is also valid. But yes, good idea.

@@ -0,0 +1,2 @@
Changed the way OAuth2 Client Credentials are provided to give the user some choice over the authentication to use.
The new parameters `--client-id` and `--client-secret` were added and `--username`, `--password` are now restricted to HTTP Basic.
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

This introduces --client-id and --client-auth. Users can now infer the
authentication to use in face of multiple available security proposals.
According to RFC6749 Section 2.3.1 all token servers are required to
support http basic auth. Instead supporting the credentials as post
data is specified as optional. Furthermore the RCF discourages using
the latter.
Auth objects provided by the pulpcli auth provider are memoized. This
way, no password needs to be written back to the pulp_ctx variable and
the oauth token can be cached in memory for the lifetime of the context.
@mdellweg mdellweg merged commit dd858b6 into pulp:main Sep 17, 2024
17 checks passed
@mdellweg mdellweg deleted the auth_power branch September 17, 2024 15:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants