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] Serve both raw (for download) and resigned (for upload) tokens #1362

Open
wants to merge 14 commits into
base: tests/mock-aai
Choose a base branch
from

Conversation

nanjiangshu
Copy link
Contributor

@nanjiangshu nanjiangshu commented Feb 6, 2025

Related issue(s) and PR(s)

This PR closes #1337

Description

This PR allows auth to serve both raw and resigned tokens as well as their corresponding s3 config files.
This PR is based on the branch tests/mock-aai in which the service mock-aai is implemented. Nevertheless, the changes of this PR and the changes in the branch tests/mock-aai are in different folders of code so there's no conflict when merging to the main branch.

How to test

  1. Start the SDA services with resignJWT enabled by
    setting AUTH_RESIGNJWT=true in the file .github/integration/sda-s3-integration.yml
    and server.jwtpubkeypath : "/shared/keys/pub/" in the config file .github/integration/sda/config.yaml.

then run

make sda-s3-up
  1. When the services are up, visit http://localhost:8801 and login with the Test User, check that there are two different tokens and also click the download buttons for two different S3 config files, that is, s3cmd-inbox.conf and s3cmd-download.conf.

  2. Modify bucket base and use_https for local testing.
    For Mac:

sed -i '' '/host_/s/inbox:8000/localhost:18000/g' s3cmd-inbox.conf
sed -i '' '/host_/s/inbox:8000/localhost:18000/g' s3cmd-download.conf
sed -i '' 's/use_https = True/use_https = False/g' s3cmd-inbox.conf
sed -i '' 's/use_https = True/use_https = False/g' s3cmd-download.conf

For Linux:

sed -i  '/host_/s/inbox:8000/localhost:18000/g' s3cmd-inbox.conf 
sed -i  '/host_/s/inbox:8000/localhost:18000/g' s3cmd-download.conf
sed -i  's/use_https = True/use_https = False/g' s3cmd-inbox.conf
sed -i  's/use_https = True/use_https = False/g' s3cmd-download.conf
  1. create test files
for i in $(seq 2); do seq $i > /tmp/t$i.txt; done

Test uploading
5. Test uploading with the config file s3cmd-inbox.conf and it should work.

sda-cli -config s3cmd-inbox.conf upload -encrypt-with-key mykey.pub.pem /tmp/t1.txt
  1. Test uploading with the config file s3cmd-download.conf and it should fail.
sda-cli -config s3cmd-download.conf upload -encrypt-with-key mykey.pub.pem /tmp/t3.txt  

Consider renewing the token.
Error: listing uploaded files: failed to list objects, reason: Unauthorized: not authorized
	status code: 401, request id: , host id: 

Test downloading
7. Start the download service by running the following command in the folder sda-download

 CONFIGFILE=dev_utils/config-notls_local.yaml go run cmd/main.go 
  1. Test accessing the download endpoint with the resigned token and it should fail with get visas failed
token=$(grep token s3cmd-inbox.conf | awk '{print $3}')
curl  -H "Authorization: Bearer $token" http://localhost:18080/metadata/datasets    
  1. Test accessing the download endpoint with the raw token and it should work (return an empty list)
token=$(grep token s3cmd-download.conf | awk '{print $3}')
curl  -H "Authorization: Bearer $token" http://localhost:18080/metadata/datasets  

@nanjiangshu nanjiangshu marked this pull request as ready for review February 6, 2025 22:01
Copy link
Collaborator

@jbygdell jbygdell left a comment

Choose a reason for hiding this comment

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

  1. Don't make changes to files in the base branch unless that one is main. Your Base branch is a branch that belongs to another PR.
  2. The root cause of all your mishaps is due to the OIDCIdentity struct having rather bad names for what data it holds. It should actually look like this:
type OIDCIdentity struct {
	Subject              string
	Passport             []string
	RawToken             string
	ResignedToken        string
	Fullname             string
	Email                string
	EdupersonEntitlement []string
	ExpDateRaw           string
	ExpDateResigned      string
}

.github/integration/sda/aai-mock/userinfos/dummy-user.yaml Outdated Show resolved Hide resolved
.github/integration/sda/aai-mock/userinfos/test-user.yaml Outdated Show resolved Hide resolved
.github/integration/sda/config.yaml Outdated Show resolved Hide resolved
sda/cmd/auth/frontend/templates/oidc.html Show resolved Hide resolved
sda/cmd/auth/main.go Show resolved Hide resolved
@nanjiangshu nanjiangshu marked this pull request as draft February 7, 2025 08:34
@nanjiangshu nanjiangshu marked this pull request as ready for review February 7, 2025 10:41
Copy link
Collaborator

@jbygdell jbygdell left a comment

Choose a reason for hiding this comment

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

remove the following commits:
ea0908d : Fix name of the test user for aai mock
d69100a : Use jwtpubkeypath instead of jwtpubkeyurl
c516ed6 : Revert "Use jwtpubkeypath instead of jwtpubkeyurl"
b78c70c : Revert "Fix name of the test user for aai mock"

Rebase this branch on main, should only be your 10 commits by then.

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