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

Add RSA SecurID server support #10667

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Add RSA SecurID server support #10667

wants to merge 4 commits into from

Conversation

milljm
Copy link

@milljm milljm commented May 14, 2021

Initial commit to aid dealing with RSA SecurID protected servers.

Requires a third party script to facilitate authentication (the pickling
of authenticated cookie): https://github.com/milljm/rsasecure_login

With everyone's blessing, I will move the 3rd party helper script over to conda-forge.
Or do this first if that is a requirement?

Initial commit to aid dealing with RSA SecurID protected servers.

Requires a third party script to facilitate authentication (the pickling
of authenticated cookie): https://github.com/milljm/rsasecure_login

With everyone's blessing, I will move the helper script over to conda-forge.
@milljm milljm requested a review from a team as a code owner May 14, 2021 17:57
@anaconda-issue-bot
Copy link

We require contributors to sign our Contributor License Agreement, and we don't have one on file for @milljm. In order for us to review and merge your code, please e-sign the PDF at https://conda.io/en/latest/contributing.html#conda-contributor-license-agreement. We then need to manually verify your signature. We will ping the bot to refresh the PR status when we have confirmed your signature.

Remove cookie missing message. The fix is the same, so post _that_ message.
@dbast
Copy link
Member

dbast commented May 17, 2021

@anaconda-issue-bot check

@anaconda-issue-bot anaconda-issue-bot added the cla-signed [bot] added once the contributor has signed the CLA label May 17, 2021
Use request headers to determine a working RSA connection.
Fix pylint issues.
@milljm
Copy link
Author

milljm commented May 19, 2021

@dbast I am not sure why the tests are not firing. Anything you can trigger?

Remove unnecessary imports
@milljm
Copy link
Author

milljm commented May 19, 2021

Please re-test.

@chenghlee chenghlee added this to the 4.10.2 milestone May 21, 2021
milljm added a commit to milljm/staged-recipes that referenced this pull request May 24, 2021
Initial commit to add rsasecure_login to conda-forge.
Note: This package requires a milestone conda/conda#10667

This is a work in progress. Aside from the obvious milstone requirement, I am not
sure I like the command name `rsasecure_login`.
milljm added a commit to milljm/staged-recipes that referenced this pull request May 24, 2021
Initial commit to add rsasecure_login to conda-forge.
Note: This package requires a milestone conda/conda#10667

This is a work in progress. Aside from the obvious milstone requirement, I am not
sure I like the command name `rsasecure_login`.
milljm added a commit to milljm/staged-recipes that referenced this pull request May 24, 2021
Initial commit to add rsasecure_login to conda-forge.
Note: This package requires a milestone conda/conda#10667

This is a work in progress. Aside from the obvious milestone requirement, I am not
sure I like the command name `rsasecure_login`.
@milljm
Copy link
Author

milljm commented May 24, 2021

Conda-Forge companion script: conda-forge/staged-recipes#15016

@@ -91,6 +94,7 @@ def __init__(self):
self.mount("https://", http_adapter)
self.mount("ftp://", FTPAdapter())
self.mount("s3://", S3Adapter())
self.mount("rsa://", SecureIDAdapter())
Copy link
Contributor

Choose a reason for hiding this comment

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

Is rsa:// a documented/standardized URL scheme component for this server?

Copy link
Author

Choose a reason for hiding this comment

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

Unfortunately not. I did some basic research on the matter using s3:// as an example. Couldn't figure out how to connect to amazon using that prefix (while using requests). So I figured I might be allowed to do this for servers protected by RSASecurID.

I realize it's pretty hacky! Suggestions?

Copy link
Author

Choose a reason for hiding this comment

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

Perhaps I should try to figure out if this server is protected by RSA, do all the things...

Copy link
Contributor

Choose a reason for hiding this comment

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

I think adding support for 2FA and other (HTTP-based) authentication mechanisms to conda is a good idea. But I want to avoid starting (or continuing, I guess) the pattern of requiring to create a custom adapter for every authn scheme, especially for schemes like this that appear to be just slightly modified HTTP requests.

I'm going to bump this out of the 4.10.2 release, so we can maybe discuss some other, more generic ways of handling HTTP-based authn.

Copy link
Author

Choose a reason for hiding this comment

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

Understood. I can try to dig around Conda's internals and see if I can come up with something on my own as well. I'll gladly post another PR if/when that occurs.

Cheers!

@chenghlee chenghlee modified the milestones: 4.10.2, Release TBD May 24, 2021
@chenghlee chenghlee added source::community catch-all for issues filed by community members tag::auth tag-features labels May 24, 2021
@kenodegard kenodegard added type::feature request for a new feature or capability and removed tag::features labels Jan 18, 2022
@travishathaway
Copy link
Contributor

Hi @milljm,

I just want to let you know that we are currently considering whether to make the CondaSession object a plugin hook (i.e. replace it with a different class). This could be an interesting use case for that.

If you are interested in that discussion, please check out the following CEP (conda enhancement proposal). It's still a draft, but if you had any input, that would be very much welcome:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed [bot] added once the contributor has signed the CLA source::community catch-all for issues filed by community members tag::auth type::feature request for a new feature or capability
Projects
Status: 🆕 New
Development

Successfully merging this pull request may close these issues.

6 participants