-
Notifications
You must be signed in to change notification settings - Fork 27
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
Implement creation of a sharing URL #257
Conversation
Consolidates configuration in the client and encrypted client that has different logic than the server for calculating the config directory.
Make the output variable be returned as part of the output.
This adds new options for the token generation, introducing an enum (replacing the prior boolean) that differentiates between sharing tokens (which should be very fine-grained) and regular read/write.
With this, one may create a sharing URL when using the OAuth2 flow.
Additionally, refactor the client sharing method to take a URL (and perform metadata discovery as needed).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some of the changes to configuration are conflicting with a recent PR Joe put together. Can you rebase?
directorUrl = configDirectorUrl | ||
} | ||
} else if objectUrl.Scheme != "" { | ||
return "", errors.Errorf("Unsupported scheme for pelican: %s://", objectUrl.Scheme) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This isn't a change request, per say, but I wonder how we could support having schemeless sharing. If I do osdf object share /chtc/staging/jhiemstra/chtc-auth-test.txt
, might it be reasonable to generate a sharing URL that points to a cache or an origin? For example, if I curl -v https://origin-auth2000.chtc.wisc.edu:1095/chtc/staging/jhiemstra/chtc-auth-test.txt?authz=<TOKEN FROM RUNNING OBJECT SHARE>
, I also get the file. We could generate this particular sharing URL when no scheme is provided.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure I'm following -- the prior conditional handles the case for schemaless sharing, no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I think I tested in a way that pointed at the wrong director, which caused a failure I misinterpreted. After pointing back to the production director, this appears to work as intended.
This fixes the case where the user provides an `osdf://`-style URL and also overrides the director URL.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This allows a user to create a "sharing URL", a URL that includes the authorization token but has minimal privileges.
The intended usage is something like this:
Includes test cases covering token generation and the director URL discovery but, honestly, it's a bit difficult to test end-to-end as the
director-cache.osg-htc.org
is not working with authenticated CHTC URLs right now.Fixes #24