-
Notifications
You must be signed in to change notification settings - Fork 11
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
Provide automatic consent for Globus #304
Conversation
a07dad7
to
22dcd47
Compare
zstash/globus.py
Outdated
def globus_flow(ep=""): | ||
CLIENT = NativeAppAuthClient( | ||
client_id="6c1629cf-446c-49e7-af95-323c6412397f", app_name="Zstash" | ||
) | ||
scopes = "urn:globus:auth:scope:transfer.api.globus.org:all" | ||
endpoint_scope = f"[ *https://auth.globus.org/scopes/{ep}/data_access ]" | ||
data_access_sopes = scopes + endpoint_scope | ||
CLIENT.oauth2_start_flow(refresh_tokens=True, requested_scopes=data_access_sopes) | ||
authorize_url = CLIENT.oauth2_get_authorize_url() | ||
print(f"Please go to this URL and login:\n\n{authorize_url}\n\n") | ||
|
||
get_input = getattr(__builtins__, "raw_input", input) | ||
auth_code = get_input("Please enter the code you get after login here: ") | ||
token_response = CLIENT.oauth2_exchange_code_for_tokens(auth_code) | ||
print(token_response) |
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.
@lukaszlacinski I'm trying to integrate @tylern4's globus_flow
script (from #298 (comment)) into zstash
. Then, users won't have to run it themselves before using zstash
with Globus.
However, I'm getting quite confused as to where it belongs. I figured it should be part of the Globus activation, so I placed the calls in the globus_activate
function of zstash/globus.py
.
- If I comment out the calls (and I have already granted consent manually), then
test_globus
works fine. - If I include the calls, the test hangs on the
self.create(use_hpss, zstash_path, cache=self.cache)
line of the test (https://github.com/E3SM-Project/zstash/blob/main/tests/test_globus.py#L170)
I'm going to keep looking into this, but I figured I should ask if you had any immediate insights on how to automate the consent granting. Thanks!
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.
From what I can tell, the auth_code = get_input("Please enter the code you get after login here: ")
line is causing the hang.
However, many lines that should be printed before that point don't print, which made me think incorrectly that the error was occurring much earlier. From what I can tell, the output, err = p.communicate()
line (https://github.com/E3SM-Project/zstash/blob/main/tests/base.py#L55) that is used to run zstash create
just doesn't print any output until the entire command completes. And that of course includes the line asking for user input.
I'm not quite sure how we would change that for testing purposes. In fact, we really don't want the unit tests to be asking for input anyway, as that would mean any test automation would also get stuck at that point. Then again, I suppose we could handle it the same way we do for endpoint activation -- have the test fail if the consents weren't granted ahead of time; once the user fixes the issue, the test can be run again.
A work-around would be to run a stand-alone globus_flow
script before running the unit tests.
As for this pull request, I guess I can try testing the automated globus_flow
functionality by manually transferring data to NERSC HPSS (from Perlmutter and from another machine).
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 ok, so it looks like we just need the globus_flow(remote_endpoint)
line below, not the globus_flow(local_endpoint)
line.
Manual test on Chrysalis:
# Create something to archive
$ emacs setup.sh
mkdir zstash_demo
mkdir zstash_demo/empty_dir
mkdir zstash_demo/dir
echo 'file0 stuff' > zstash_demo/file0.txt
echo '' > zstash_demo/file_empty.txt
echo 'file1 stuff' > zstash_demo/dir/file1.txt
$ chmod 755 setup.sh
$ ./setup.sh
# Activate environment with `globus_flow` included in `zstash`
$ conda activate zstash_dev_n298
$ zstash create --hpss=globus://nersc/~/n298_chrysalis_dev zstash_demo
# On Perlmutter:
$ hsi
$ cd n298_chrysalis_dev
$ ls
# 000000.tar index.db
# Success
# Now, uncomment the `globus_flow(remote_endpoint)` line, and run `pip install .`
$ zstash create --hpss=globus://nersc/~/n298_chrysalis_dev_v2 zstash_demo
# This now asks for the consent (even though it should still be granted from an earlier run...).
# Once I enter the code, it completes.
# On Perlmutter:
$ hsi
$ cd n298_chrysalis_dev_v2
$ ls
# 000000.tar index.db
# Success
748e8ee
to
1e028df
Compare
@tylern4 @lukaszlacinski Could you review this pull request, please? I tested with the following two lines:
My one remaining problem is I can't be entirely sure everything is working properly until I can disable currently granted consents. Nothing I do seems to actually accomplish this.
@golaz Would it be better to have the user include a @xylar @chengzhuzhang This is the pull request I would like to include in the immediate |
@forsyth2, @tylern4, @lukaszlacinski: I'm not sure I fully understand the complication here. But I'm concerned that we are going to confuse users for no good reason if we have to add a command line argument to either opt-in or opt-out. If we go with opt-in, users probably won't bother to figure out what they need to do and they'll complain that If we go with opt-out, users need to constantly add an additional command line argument which just complicates the There must be a way we can determine if consent is required, and walk users through the steps only when that is needed, no? |
Okay I looked through your code a bit more and I recommend against the flags, and removing my globus_flow code since it looks like you're using a module ( What you can do is wrap your transfers in a try:
task = transfer_client.submit_transfer(transfer_data)
except TransferAPIError as err:
if err.info.consent_required:
native_client = NativeClient(
client_id="6c1629cf-446c-49e7-af95-323c6412397f", app_name="Zstash")
native_client.login(
requested_scopes=f"urn:globus:auth:scope:transfer.api.globus.org:all[ *https://auth.globus.org/scopes/{remote_endpoint}/data_access ]") The problems to solve though is multifold, since it may not always be the def check_endpoint_version_5(transfer_client, ep_id):
logging.debug(f"Checking {ep_id}")
output = json.loads(transfer_client.get_endpoint(ep_id))
logging.debug(f"Version is {output['gcs_version']}")
if int(output['gcs_version'].split('.')[0]) >= 5:
return True
else:
return False You can also get more than one f"urn:globus:auth:scope:transfer.api.globus.org:all[ *https://auth.globus.org/scopes/{remote_endpoint}/data_access *https://auth.globus.org/scopes/{local_endpoint}/data_access ]" |
@tylern4 Thanks for the updates! I've included them in the latest commit. How do the changes look now? Re: #298 (comment) -- thank you, I was able to rescind consents, and I did get prompted for a new consent. However, I notice that I'm sometimes able to successfully transfer files even after rescinding a consent. Is rescinding a consent not instantaneous? Is there some sort of lag time? For reference, I've tested using the following scripts. Chrysalis:
Perlmutter:
|
I'm not sure how long the consents take to propagate but my experience with other Globus actions is that it may take a few seconds-minutes. I added some changes in #306 to check both endpoints versions and get the consents. It's needed if both endpoints are v5 like the original NERSC HPSS to NERSC Perlmutter transfer. The one note is that it will quit after an initial run and ask the user to submit another. I believe this is because it would need to re-new the |
@tylern4 Thanks for your edits in #306. I ran zstash with them, but everything still works as if I have granted consents (even though I rescinded them yesterday). Is there some sort of hidden directory/cache on the machine? Am I missing something on the web page? @mahf708 @golaz @chengzhuzhang @tomvothecoder I'm really trying to get this merged (by end of week), so it can be included in the upcoming patch release of E3SM Unified. I feel like the always-having-consents-granted problem may be specific to me. If any of you have time, can you please try running the following? On Chrysalis:
On Perlmutter:
It's not ideal that the test just hangs, but when GitHub Actions run the unit tests, it skips the tests requiring HPSS (including the Globus test), so it at least won't cause the build/release to fail. To get around the hang, you'd need to run the stand-alone |
testing now, will update with results momentarily:
I noticed that in your steps you don't activate the environment you create. That is,
|
Do you know where the auth get stored btw? Is it ~/.local or ~/.config or is inside $CONDA_PREFIX somewhere? |
Sorry about that, thanks. I already had mine activated so I forgot that step when writing up the general directions.
No, but perhaps that may be causing the always-have-consent problem |
Yes, that's where my consents appear as well. Did you delete just the most recent one ("Zstash Login Test") or all of them? (I've only been deleting the recent ones that I just granted). It's interesting that you get
Are you logged into LCRC and NERSC on Globus? Perhaps you're missing some credentials. |
@forsyth2 @mahf708 Your Globus tokens are stored in So for testing I've been removing all the consents, I put a small fix in this morning to the and the two tests I've been running have been working from Perlmutter. The pytests don't seem to return the second prompt for auth which is why they hang.
|
Once I provided the consent, it worked (and I got two email notifications with SUCCEEDED in all caps lol). I only deleted the last one (Zstash Login Test) because I wanted to only play with the newly added one. This is from Chrysalis, but I on globus I am logged on both LCRC and NERSC. I tried tracing where the auths are stored but I couldn't get anywhere. I suppose it is doing a lot of encryption. I checked |
Ah, we wrote at the same time. The creds are encrypted in ~/.globus-native-apps.cfg. Makes sense. |
I ran the following. Everything now appears to work correctly for me! Thank you @tylern4 and @mahf708!! I think I can actually proceed with merging this now. A manual test:
The unit test:
|
* Update data_access consent check * None should return False
@tylern4 I'm wondering if we actually need this pull request. I noticed you had I then tried running with the latest official release of Unified, and it remarkably prompted for consent. I'm assuming either 1) the conda build picked up a very new version of a dependency that handles the consents, 2) removing I feel like it has to be the latter case, since I haven't rebuilt the development environments today but I couldn't get the code working this morning. Then again, we were running into Is there a case we do need the changes in this pull request then? Unit test:
Manual test:
|
@forsyth2 I'm not sure why you can't remove the consents, maybe you're the owner of the client_id so the consents are different? When I do and try it does go to the sys.exit. If I instead replace the diff --git a/zstash/globus.py b/zstash/globus.py
index 5d51f36..c3a057f 100644
--- a/zstash/globus.py
+++ b/zstash/globus.py
@@ -65,10 +65,7 @@ def submit_transfer_with_checks(transfer_data):
native_client.login(
requested_scopes=scopes
)
- # Quit here and tell user to re-try
- print("Consents added, please re-run the previous command to start transfer")
- sys.exit(0)
- # I think what's happening is that it needs to reload the token with the new consents
+ task = transfer_client.submit_transfer(transfer_data)
else:
raise err
return task I get (starting from the first
The authorization fails because the |
Stepping through with On the manual test (on Chrysalis):
I thought I was successfully removing the consents. After all, everything worked fine in #304 (comment) and strangely, using the latest E3S Unified, in #304 (comment).
Hmm yeah I don't seem to ever get there. I just don't understand why it's behaving differently. Also re: the |
@lukaszlacinski Can you please review these comments starting at the 4th above this one (#304 (comment)). I think this is probably ready to merge, but it bothers me that I can't seem to get consistent behavior nor behavior matching what @tylern4 gets. |
Consents are per client and per scope. Tokens and consents (if have been rescinded or never given) for different scopes are requested in two different places in
Please check the log record
in the last @tylern4 comment. Scopes are specified in one string with a space as a separator. If data access scopes for GCSv5 mapped collections are added to scopes, that the For example four scopes: First scope to talk to the Globus Auth API, second scope to talk to the Globus Transfer API, and two dependent scopes for Globus Transfer to talk to zstash source and destination GCSv5 mapped collections on behalf of a user. |
@lukaszlacinski Sorry, I'm still confused about what these scopes mean for the difference in behavior between myself and @tylern4. Am I supposed to be rescinding more consents/authentications than I currently am between tests? |
Latest tests: ChrysalisI notice that the Chrysalis endpoint (https://app.globus.org/file-manager/collections/61f9954c-a4fa-11ea-8f07-0a21f750d19b/overview) has a warning: "This endpoint must be migrated to Globus Connect Server version 5 by December 18, 2023 for continued support. ". So, it looks like it hasn't been updated to need consents yet. I wonder if that's why I was getting different behavior there. Current Unified:
Using this pull request:
PerlmutterCurrent Unified:
Using this pull request:
ConclusionsThe full logic of authentication, scopes, and consents is significantly more complicated than I would like. Luckily, it appears I now have my side working such that the current Unified fails but these changes succeed, thus matching results of others. I will go ahead and merge these changes, and start a release candidate for a Thank you for your help @tylern4 @lukaszlacinski. |
Provide automatic consent for Globus. Resolves #298 (alternative approach to #300).