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

DIG-1902: check if user is already authz #41

Merged
merged 1 commit into from
Jan 28, 2025
Merged

Conversation

daisieh
Copy link
Member

@daisieh daisieh commented Jan 25, 2025

Do an extra tiny check: if the user is already a CanDIG-authorized user, return 200 and don't add the user to the pending list.

To test:
Using a site admin token, try adding the site admin to the pending list:

curl -X "POST" "http://candig.docker.internal:5080/ingest/user/pending/request" \
     -H 'Authorization: Bearer <site admin token>'

You should get 200 "User [email protected] is already a CanDIG authorized user"

If you check the pending users list:

curl "http://candig.docker.internal:5080/ingest/user/pending" \
     -H 'Authorization: Bearer <site admin token>'

the result should not include the site admin.

@daisieh daisieh force-pushed the daisieh/ingest-auth branch from 662b0cc to 7b95e01 Compare January 25, 2025 00:17
@daisieh daisieh requested a review from OrdiNeu January 25, 2025 00:18
@daisieh daisieh force-pushed the daisieh/ingest-auth branch from 82e20e4 to 7b95e01 Compare January 25, 2025 00:43
@OrdiNeu
Copy link
Contributor

OrdiNeu commented Jan 27, 2025

Not currently able to get this to work -- would I test it by editing lib/candig-ingest/candigv2-ingest/requirements.txt with:

candigv2-authx@git+https://github.com/CanDIG/candigv2-authx.git@daisieh/ingest-auth

and running the commands you posted? If so I'm not able to replicate it.

@daisieh
Copy link
Member Author

daisieh commented Jan 27, 2025

You should be able to recompose ingest with the authx requirement as you mentioned; maybe use BUILD_OPTS="--no-cache" just to make sure that the changes are picked up, and then run the tests.

@OrdiNeu
Copy link
Contributor

OrdiNeu commented Jan 27, 2025

Still not getting the 200 message and the Site admin is still being added to the pending users list, even after being accepted.

@daisieh
Copy link
Member Author

daisieh commented Jan 28, 2025

I wonder if there is something funky about the various lists in your setup at this point...is this based on a clean install? When you run make recompose-candig-ingest, is the line >> approving [email protected] as a CanDIG authorized user followed by any errors? And is the commit being pulled for candigv2-authx the one expected, 7b95e01?

Check the status of the user before you run the test:

  • Is there anything in the pending list before?
  • If you run http://candig.docker.internal:5080/ingest/user/me with the site admin token, do you get a 200 and a user json, meaning that the user is a CanDIG-authorized user?

Copy link
Contributor

@OrdiNeu OrdiNeu left a comment

Choose a reason for hiding this comment

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

Mysteriously started working this morning. Unsure what changed, given how many times I rebuilt it yesterday with caches disabled.

@daisieh daisieh merged commit 7840a56 into develop Jan 28, 2025
2 checks passed
@daisieh daisieh deleted the daisieh/ingest-auth branch January 28, 2025 18:35
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.

2 participants