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.

Sign up for GitHub

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

Jump to bottom

Check endpoint validity for pre-signed URLs #1027

Merged
merged 1 commit into from
Nov 25, 2024
Merged

Check endpoint validity for pre-signed URLs #1027

merged 1 commit into from
Nov 25, 2024

Conversation

smallhive
Copy link
Contributor

Closes #1025.

@smallhive
Copy link
Contributor Author

I have a question. Do we need to have generate-presigned-url inside neofs-s3-authmate?

My concern is about the internal command:

./neofs-s3-authmate generate-presigned-url \
  --endpoint 'http://localhost:19080' \
  --method 'GET' --bucket 'heh1732164910' \
  --object 'file-1732164943.txt' \
  --lifetime '3600s' \
  --aws-secret-access-key '48966f87c37c12c15548a30ef3c1319438b9364a11ca033bea6fc4e2eb5565de' \
  --aws-access-key-id '78Tx3pRmSXNryyVUA7N1WCxg8JZWaGcR2G5M8AWbWuur04iSdjuV8LM51duiiSgY8KqyuMTx4XFLEMfWPtC9fkWLo'

It easily can be replaced with native AWS CLI:

aws s3 presign s3://heh1732164910/file-1732164943.txt --expires-in 3600 --endpoint-url localhost:19080

and leads to the same result. Of course, aws configure used the same access and secret keys.

Don't you mind if we remove this generate-presigned-url from neofs-s3-authmate in the separate issue?
@roman-khimov

@roman-khimov
Copy link
Member

I'm not sure why it was added in the first place. If aws can do it and we're compatible, what's the point of having this command? Simplify testing? @evgeniiz321, do you need it? In general authmate is expected to handle authboxes and not much else, duplicating aws cli is the last thing we want.

roman-khimov
roman-khimov reviewed Nov 21, 2024
View reviewed changes
api/auth/center.go Outdated
@@ -314,7 +315,7 @@ func cloneRequest(r *http.Request, authHeader *authHeader) *http.Request {

func (c *center) checkSign(authHeader *authHeader, box *accessbox.Box, request *http.Request, signatureDateTime time.Time) error {
awsCreds := credentials.NewStaticCredentials(authHeader.AccessKeyID, box.Gate.AccessKey, "")
signer := v4.NewSigner(awsCreds)
signer := v4amz.NewSigner(awsCreds)
Copy link
Member

@roman-khimov roman-khimov Nov 21, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Why was it added initially?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Our implementation?

Copy link
Member

Choose a reason for hiding this comment

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

Yes.

Copy link
Member

Choose a reason for hiding this comment

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

#528

Copy link
Member

Choose a reason for hiding this comment

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

Maybe #1028 can help here.

@roman-khimov
Copy link
Member

Not a lot of traces left. authmate command was just added in #531, that's it.

@evgeniiz321
Copy link

@roman-khimov nope, i don't need the authmate generate-presigned-url. Using aws cli/boto3 seems to be the right way to go.

@smallhive smallhive force-pushed the 1025-cant-get-object-with-presigned-url branch from 03b2976 to d817ab3 Compare November 25, 2024 09:20
@smallhive
Copy link
Contributor Author

I left only a fix for presign URLs. Internal AWS implementation displaced to another PR

@roman-khimov roman-khimov merged commit 9c44b1a into master Nov 25, 2024
15 of 18 checks passed
@roman-khimov roman-khimov deleted the 1025-cant-get-object-with-presigned-url branch November 25, 2024 11:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@roman-khimov roman-khimov roman-khimov approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Can't get object with presigned url
3 participants
@smallhive @roman-khimov @evgeniiz321