Skip to content

Conversation

@AK2000
Copy link
Contributor

@AK2000 AK2000 commented Aug 20, 2025

Description

The globus auth client id is the wrong field to use to validate the user. Instead, we change to use the sub which is unique to each globus identity.

Fixes

Type of Change

  • Bug (fixes for a bug or issue)
  • Security (security related changes)

Testing

Changed the testing of authenticate to use the username.

Pull Request Checklist

Please confirm the PR meets the following requirements.

  • Tags added to PR (e.g., breaking, bug, enhancement, internal, documentation, package, development, security).
  • Code changes pass pre-commit (e.g., mypy, ruff, etc.).
  • Tests have been added to show the fix is effective or that the new feature works.
  • New and existing unit tests pass locally with the changes.
  • Docs have been updated and reviewed if relevant.

@AK2000 AK2000 requested a review from gpauloski August 20, 2025 15:14
@gpauloski gpauloski added the bug Error, flaw, or fault that causes unexpected behavior label Aug 21, 2025
@gpauloski
Copy link
Collaborator

gpauloski commented Aug 21, 2025

Oh boy---thanks for finding this 😅.

I think my intention was to use a unique field that wasn't the username since the username has some caveats. The username will be for their effective identity (i.e., for their default identity). This might cause issues if authenticated with one identity (e.g., UChicago) on machine A and a different identity (e.g., NERSC) on machine B.

Maybe doing an intersection of identity_sets would be better? Or maybe this isn't an issue and username reliable?

@AK2000
Copy link
Contributor Author

AK2000 commented Aug 21, 2025

Hmm, I haven't tried it, but I thought username would be fine. From the SDK docs, the username is derived from the sub field, and the sub field will be the "primary identity of the account" unless a specific identity is requested by the client. Since the Proxystore client doesn't request a specific identity provider, I think it should always be the primary identity, regardless of how a user authenticates.

@AK2000 AK2000 changed the title Changing globus user auth to use username Changing globus user auth to use sub Aug 21, 2025
@AK2000
Copy link
Contributor Author

AK2000 commented Aug 21, 2025

I switched this PR to use sub based on the recommendation from the Globus auth team. sub is unique to a Globus user identity, while username is only unique at a certain point in time.

@gpauloski gpauloski force-pushed the ak2000-issue714-fix branch from 6a07dcb to 1a36d45 Compare August 25, 2025 19:20
@gpauloski gpauloski changed the title Changing globus user auth to use sub Change Globus user auth to use sub field Aug 25, 2025
@gpauloski
Copy link
Collaborator

Thanks for figuring that out! Will make a release and update the prod relay server this week.

@gpauloski gpauloski merged commit b30d5d7 into main Aug 25, 2025
12 checks passed
@gpauloski gpauloski deleted the ak2000-issue714-fix branch August 25, 2025 19:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Error, flaw, or fault that causes unexpected behavior

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Authenticate on Relay Server is Broken

3 participants