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

Auth client-credentials issues #430

Closed
shanedell opened this issue Nov 19, 2024 · 7 comments · Fixed by #444
Closed

Auth client-credentials issues #430

shanedell opened this issue Nov 19, 2024 · 7 comments · Fixed by #444

Comments

@shanedell
Copy link
Contributor

shanedell commented Nov 19, 2024

Multiple issues found:

  1. After release 0.9.0, the auth client-credentials command no longer supports --client-id and --client-secret.

  2. --with-client-creds and --with-client-creds-file not stopping the prompt from coming up.

    • When running:
       go run auth client-credentials \
           --host http://localhost:8080 \
           --with-client-creds '{"clientId":"opentdf","clientSecret":"secret"}'
    • The below screenshot is gotten:
    Screenshot 2024-11-19 at 5 25 36 PM
    • The same thing happens using --with-client-creds-file with that same content in the file
  3. After getting past entering the data, if running keycloak in docker I always get this error:

    ERROR    could not authenticate: failed to get platform configuration: Get "http://keycloak:8888/auth/realms/opentdf/.well-known/openid-configuration": dial tcp: lookup keycloak: no such host
    • This is even when setting --host http://localhost:8080.

When testing I tested all releases from that had name otdfctl and only 0.6.0 - 0.9.0 worked properly for me, number 2 wasn't really tested though with these ones but in later releases it was.

@shanedell
Copy link
Contributor Author

I'd be willing to submit a PR to help with these issues.

However, would 1 be solved by:

  1. Updating the docs to reference with --with-client-creds and/or --with-client-creds-file
  2. Adding support for --client-id and client-secret back in?

@jakedoublev
Copy link
Contributor

Hi @shanedell, thanks for raising this! The best fix is to remove the reference to those flags in the documentation/manual. There are a few reasons for this:

  1. Passing secrets into the shell via argument or flag value leaks the secret into shell history.
  2. The auth client-credentials command is intended to be a person entity user flow where the credentials are passed in interactively so they are not leaked into shell history and are saved to the OS keyring under the profile. Most person users will also be better off with the auth login browser-driven OAuth flow instead.
  3. If client credentials are utilized to authenticate the CLI as an automation rather than a person user, the more secure option that avoids leaking the secret into shell history is --with-client-creds-file. In an automation scenario there is no need for a profile and keyring/profile support on Linux is untested.

Longer-term, it is my opinion that --with-client-creds should be deprecated completely (in favor of solely --with-client-creds-file) so that there is no scenario where a client secret can be found in shell history, but this is subject to change.

@jakedoublev
Copy link
Contributor

See related issue: #417

@shanedell
Copy link
Contributor Author

shanedell commented Nov 20, 2024

@jakedoublev When updating the docs/man/auth/client-credentials.md file and the platform docs to remove references of --client-id and --client-secret. Should I also delete the below lines from docs/man/auth/client-credentials.md:

  args: 
    - client-id
  arbitrary_args:
    - client-secret

or what do these mean currently?


For my 3rd issue would a possible fix be allowing the opentdf platform sdk to allow setting the keycloak host? Then adding a flag to this CLI that gets passed to the sdk.New where the sdk will update endpoints based on this value? Asking because if you follow the guide here https://github.com/opentdf/platform/blob/main/Consuming.md you will get the same error I got in item 3. This is due to when fully running in docker the issuer host needs to be the keycloak docker service for some of the other containers. However, when running the CLI this default of trying to hit keycloak causes the error mentioned so it would be nice to be able to reroute to localhost instead.

@jakedoublev
Copy link
Contributor

Ah, @shanedell I did not remember we had added arguments for the client id and secret. Are you able to authenticate your profile with ./otdfctl auth client-credentials opentdf secret assuming you have not changed the default client ID and secret provisioned into the platform's keycloak container?

I believe the third issue you experienced is not addressable here within otdfctl, but seems like it is due to doc drift in the opentdf platform repo as docker compose up appears to starts keycloak at http://localhost:8888.

@shanedell
Copy link
Contributor Author

@jakedoublev Yes I was able to authenticate with ./otdfctl auth client-credentials opentdf secret. I will create a PR to update the docs.

As for the 3rd issue should open an issue in the platform repo that gives more detail?

@shanedell
Copy link
Contributor Author

@jakedoublev I have created a new issue for the platform here: opentdf/platform#1785. I will create a PR soon to update the information here with client-credentials.

shanedell added a commit to shanedell/otdfctl that referenced this issue Nov 25, 2024
shanedell added a commit to shanedell/otdfctl that referenced this issue Nov 25, 2024
shanedell added a commit to shanedell/otdfctl that referenced this issue Nov 25, 2024
shanedell added a commit to shanedell/otdfctl that referenced this issue Dec 2, 2024
shanedell added a commit to shanedell/otdfctl that referenced this issue Dec 3, 2024
shanedell added a commit to shanedell/otdfctl that referenced this issue Dec 3, 2024
jakedoublev pushed a commit that referenced this issue Dec 3, 2024
docs: Update documentation on how to run client-credentails

Closes #430

Signed-off-by: Shane Dell <[email protected]>

Signed-off-by: Shane Dell <[email protected]>
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 a pull request may close this issue.

2 participants