Add new pelican token create tool#2314
Merged
Merged
Conversation
jhiemstrawisc
added
enhancement
h2zh
requested changes
May 21, 2025
h2zh
left a comment
h2zh
left a comment
Contributor
There was a problem hiding this comment.
This is indeed a mega pull request, which not only contains the new command, but also ships an overhaul of the current token scope.
For the scopes with new names, would there be a backward compatibility issue? For example, when an old client creates a token with old scope name, could the new server recognize and verify it?
There are other smaller issues in the comments below.
We previously only had scopes for WLCG-style tokens. This adds Scitokens scopes and hooks them up to the `.Path()` function so we can generate a scope like `read:/foo` for scitokens and `storage.read:/foo` for WLCG.
WLCG and Scitokens encode their various scopes differently. For example,
a scitoken for reading `/foo` requires the `read:/foo` scope, while the
equivalent WLCG token requires the `storage.read:/foo` scope.
These new interfaces make it easier to parse some profile and then use
the `profile.ReadScope("/foo")` call to generate the correct scope.
aowen-uwmad
reviewed
May 27, 2025
aowen-uwmad
left a comment
aowen-uwmad
left a comment
Contributor
There was a problem hiding this comment.
One suggestion for the help text.
I'll let Howard decide when to approve the full PR.
h2zh
requested changes
May 27, 2025
h2zh
left a comment
h2zh
left a comment
Contributor
There was a problem hiding this comment.
A security concern and other trivial comments
The function in question is one that checks whether a given issuer URL provides a public key whose KID field matches that of a given private key. I decided not to use "verify" in the name, because that word is loaded and often connotes some kind of cryptographic operation. Instead, this function only aims to make sure the user generating a token with the new `pelican token create` function doesn't use an issuer that couldn't possibly work.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The most basic form of the command might look like:
which will create a read token for the resource at the specified level (the object path trimmed of the namespace prefix).
For example, assuming I had the signing key for
pelican://osg-htc.org/chtc/, I could create a token that grants read/write access to my entire section of staging with:Issuer determination happens by querying the Director and checking any locally-discovered (or provided) keys against the keys hosted by the director-discovered issuer URLs.
If the provided pelican URL can't be parsed or we fail to look up Director info for it, the user should still be able to bypass the errors by providing the needed information explicitly.
Note that the linked issue said failure to provide scopes should fall back to
--read. I ultimately decided against this because I like the idea of forcing users to explicitly understand what capabilities their tokens contain. However, if the user provides no scopes, a warning is printed with a suggestion of which scopes are available.Finally, this still allows for creation of totally arbitrary tokens as the old
pelican origin token createutility did, even if the mechanism is a bit clunky. I'm okay with that for the time being, because I don't foresee anyone other than power users or developers wanting to use that (for now). After merging this, I'll open a followup issue to revisit it. For example, generation of alot.deletetoken doesn't really need an input pelican URL, but for the time being the tool will still require some resource URL. The invocation might look like:this works because explicitly providing an issuer and a scope (the two values we'd otherwise seek to gain by querying the Director) lets us ignore the fact that there is no
some-path.comfederation.