Skip to content

adding support for SciTokens https://scitokens.org/ - #69

Merged
JasonAlt merged 26 commits into
XSEDE:masterfrom
scitokens:master
Jul 24, 2020
Merged

adding support for SciTokens https://scitokens.org/#69
JasonAlt merged 26 commits into
XSEDE:masterfrom
scitokens:master

Conversation

@ag012

@ag012 ag012 commented Jan 7, 2020

Copy link
Copy Markdown
Contributor

No description provided.

@jbasney jbasney left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Thanks @ag012! I submitted a corresponding XSEDE activity for this feature at https://software.xsede.org/display/xci-694.

BuildRequires: checkpolicy
BuildRequires: policycoreutils-python
BuildRequires: python-setuptools
BuildRequires: scitokens-cpp

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The scitokens-cpp package can be built from source (https://github.com/scitokens/scitokens-cpp) or RPMs are available from OSG (https://opensciencegrid.org/docs/common/yum/) and soon from EPEL (https://src.fedoraproject.org/rpms/scitokens-cpp).

Comment thread server/README.md

Payload format for Scitokens:
"scope": “ssh:username",
"aud": "demo.scitokens.org"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I think the audience (aud) is the hostname of the SSH server, based on the gethostname() call in scitoken_verify().

@ag012

ag012 commented Jan 14, 2020

Copy link
Copy Markdown
Contributor Author

What is auth_method? Is it already split by space?
auth_method is a member of config. config is a struct preprocessed in config.c

Probably just need this to be "break", since it will break out of the loop right to the goto statement.
Thanks for the comment. I've change that to break.

@ag012

ag012 commented Jan 14, 2020

Copy link
Copy Markdown
Contributor Author

Note that rewrite _cmd_login 78f9c13 added a non-optional configuration field and it should be mentioned outside of the scitokens section in readme

@rpwagner

Copy link
Copy Markdown

Thanks @ag012! I submitted a corresponding XSEDE activity for this feature at https://software.xsede.org/display/xci-694.

@jbasney this may be both overly pedantic and not be the right place to discuss this, but on the XSEDE activity the term "proprietary" seems inappropriate for Globus Auth OAuth 2.0 Tokens. They're opaque bearer tokens based on IETF RFC 7662. Opaque is not the same as proprietary.

@jbasney

jbasney commented Jan 30, 2020

Copy link
Copy Markdown

@jbasney this may be both overly pedantic and not be the right place to discuss this, but on the XSEDE activity the term "proprietary" seems inappropriate for Globus Auth OAuth 2.0 Tokens. They're opaque bearer tokens based on IETF RFC 7662. Opaque is not the same as proprietary.

I'm happy to be corrected. It's my understanding that only tokens issued by globus.org are currently accepted by oauth-ssh, but I agree it should be possible to support any issuers of opaque bearer tokens that comply with RFC 7662 by adding a few new oauth-ssh configuration options. I opened issue #70 for it. In that case, I'd agree the term "proprietary" would no longer be appropriate.

In any case, I removed "proprietary" from the description at https://software.xsede.org/display/xci-694.

@rpwagner

rpwagner commented Jan 30, 2020

Copy link
Copy Markdown

Thanks, Jim. I'll contribute to #70. Well, participate, at least.

@JasonAlt

Copy link
Copy Markdown
Collaborator

If this is ready approved and ready for merge, it'll need the 'draft' flag changed.

@jbasney

jbasney commented Jul 22, 2020

Copy link
Copy Markdown

Who can remove the draft flag? Here's what I see:

image

@ag012
ag012 marked this pull request as ready for review July 22, 2020 15:45
@JasonAlt
JasonAlt merged commit ed8aee6 into XSEDE:master Jul 24, 2020
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.

5 participants