Skip to content

Conversation

@jreiberkyle
Copy link
Contributor

@jreiberkyle jreiberkyle commented Dec 2, 2022

Related Issue(s):

Closes #808 and #699 and #396

Proposed Changes:

For inclusion in changelog (if applicable):

  1. CLI now obtains authorization from secret file exclusively.
  2. Add planet auth store to allow storing authorization in the secret file. This command raises a confirmation prompt.
  3. When planet auth store or planet auth init are used and the authorization environment variable exists, a warning message is printed with instructions for how to update the environment variable, if desired
  4. auth.Auth.write() renamed to auth.Auth.store() to match CLI command

Not intended for changelog:

  1. CliSession() now manages authorization information in addition to headers instead of managing authorization information in Click context. This has the benefit of only obtaining authorization when a CliSession() is created so not every time something like planet data --help is called.
  2. Add tests for CLISession() to ensure auth information is obtained from the file and that expected headers are sent.
  3. Update docs on CLI and SDK authorization priorities. I still think that CLI authorization needs its own section, similar to SDK, so this information can be looked up quickly.

Diff of User Interface

Scenario 1: Authorization environment variable exists with value ENVAPIKEY and config file exists with value CONFIGAPIKEY

Old behavior:

$ planet auth value
ENVAPIKEY

New behavior:

$ planet auth value
CONFIGAPIKEY

Scenario 2: Authorization environment variable exists. It is desired to update the config file

Old behavior:

$ planet auth init
Email: email
Password: 
Initialized

New behavior:

$ planet auth init
Email: email
Password: 
Initialized
Warning - Environment variable PL_API_KEY already exists. To update, with the new value, use the following:
export PL_API_KEY=$(planet auth value)

Scenario 3: It is desired to update the config file authorization from the environment variable.

Old behavior:

from planet.auth import Auth

Auth.from_env().write()

New behavior:

from planet.auth import Auth

Auth.from_env().store()

PR Checklist:

  • This PR is as small and focused as possible
  • If this PR includes proposed changes for inclusion in the changelog, the title of this PR summarizes those changes and is ready for inclusion in the Changelog.
  • I have updated docstrings for function changes and docs in the 'docs' folder for user interface / behavior changes
  • This PR does not break any examples or I have updated them

(Optional) @mentions for Notifications:

@jreiberkyle jreiberkyle marked this pull request as ready for review December 2, 2022 19:34
@jreiberkyle
Copy link
Contributor Author

@cholmes can you review the UI changes?
@JuLeeAtPlanet can you review the doc changes?
@kevinlacaille can you review the code implementation?

ty!

@jreiberkyle
Copy link
Contributor Author

actually, changing review structure so it's just on @kevinlacaille for the implementation. And @cholmes and @JuLeeAtPlanet FYI on the docs and UI

@jreiberkyle jreiberkyle changed the title Auth 808 Clarify auth source, CLI only sources from config file Dec 3, 2022
@jreiberkyle jreiberkyle changed the title Clarify auth source, CLI only sources from config file Clarify auth source, CLI only sources from config file, add planet auth config Dec 3, 2022
@jreiberkyle jreiberkyle changed the title Clarify auth source, CLI only sources from config file, add planet auth config Clarify auth source, CLI only sources from config file, add planet auth store Dec 3, 2022
@jreiberkyle jreiberkyle changed the title Clarify auth source, CLI only sources from config file, add planet auth store Clarify auth source, CLI only sources from config file, add planet auth store, , Auth.write() -> Auth.store() Dec 3, 2022
@jreiberkyle jreiberkyle changed the title Clarify auth source, CLI only sources from config file, add planet auth store, , Auth.write() -> Auth.store() Clarify auth source, CLI only sources from config file Dec 3, 2022
@jreiberkyle jreiberkyle merged commit b9dc4ae into main Dec 3, 2022
@jreiberkyle jreiberkyle deleted the auth-808 branch March 15, 2023 19:44
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.

Clarify auth source with CLI only sourcing from config file and additional log messages

2 participants