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

GAIA-26824 Remove the get_access_token from sdk and --print_token from cli #388

Merged

Conversation

prabesh-paudel
Copy link
Contributor

@prabesh-paudel prabesh-paudel commented Dec 22, 2023

Goal/Problem:
With auth0, we no longer allow people to specify username and password to get their access token.
So get_access_token function, that allows people to use their username and password to fetch their access token needs to be removed.

We can see no calls to /api-token from non webapp origin with username and passwd for last 4 days. Kibana.

Solution.
Remove the get_access_token function from sdk and --print_token argument from cli.

Testing
Tested importing get_access_token: from groclient.lib import get_access_token throws an error as expected.
Tested gro_client:

python -m groclient --item=sesame --region=ethiopia works. (env variable is set)
python -m --item=sesame --region=ethiopia  --user_email prabesh throws error as expected
python -m --item=sesame --region=ethiopia  --print_token throws error as expected.
python -m --item=sesame --region=ethiopia  --token ... works.
python -m groclient --item=sesame --region=ethiopia without token gives error as expected. AssertionError: Need --token, or the access token to be set in environment variable: $GROAPI_TOKEN

@prabesh-paudel prabesh-paudel marked this pull request as ready for review January 8, 2024 15:21
@prabesh-paudel prabesh-paudel requested a review from a team as a code owner January 8, 2024 15:21
Copy link

@daire-gro daire-gro left a comment

Choose a reason for hiding this comment

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

looks good to me! are there any automated tests that we need to update in this repository?

@daire-gro
Copy link

@prabesh-paudel there are two other places that seem to mention 'user_email' param

README.md
46: gro_client --metric="Production Quantity mass" --item="Corn" --region="United States" --user_email="[email protected]"

docs/quick-start-projects/gro_client-tool.rst
8: gro_client --metric="Production Quantity mass" --item="Corn" --region="United States" --user_email="[email protected]"

@prabesh-paudel
Copy link
Contributor Author

@prabesh-paudel there are two other places that seem to mention 'user_email' param

README.md 46: gro_client --metric="Production Quantity mass" --item="Corn" --region="United States" --user_email="[email protected]"

docs/quick-start-projects/gro_client-tool.rst 8: gro_client --metric="Production Quantity mass" --item="Corn" --region="United States" --user_email="[email protected]"

Great catch. I just updated it. Thank you !!

looks good to me! are there any automated tests that we need to update in this repository?

I can't find any more references to the get_access_token or user_email and the unit test seems to pass, so I guess no..

@prabesh-paudel prabesh-paudel merged commit 341cc7f into development Jan 9, 2024
3 checks passed
@prabesh-paudel prabesh-paudel deleted the GAIA-26824-remove-get-access-token-from-sdk branch January 9, 2024 14:38
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.

3 participants