Skip to content

Conversation

@mortenpi
Copy link
Member

PkgAuthentication doesn't actually check if the token in auth.toml works, but JuliaHub.jl does (by calling /api/v1). So this makes JuliaHub.authenticate fall back to force=true if it detects that the token auth.toml is actually invalid (but is not expired).

This can happen if the server invalidates the token for some reason. In a nutshell, it treats that case the same as if the token is expired.

I didn't really see an easy way to mock-test the main logic in _authenticate without a lot of refactoring. But it is working locally:

julia> auth = JuliaHub.authenticate("juliahub.com")
┌ Warning: Existing token appears invalid, retrying with `force=true`.
│ Existing auth.toml backed up in: /home/mortenpi/.julia/servers/juliahub.com/auth.toml.25d828ea.backup
└ @ JuliaHub ~/jc/juliahubjl/JuliaHub.jl/src/authentication.jl:289
Authentication required: please authenticate in browser.
The authentication page should open in your browser automatically, but you may need to switch to the opened window or tab. If the authentication page is not automatically opened, you can authenticate by manually opening the following URL: https://juliahub.com/auth/response?1asdu9b32c91b397a9a10vbr2f08a63654
JuliaHub.Authentication("https://juliahub.com", "mortenpi", *****)

@mortenpi mortenpi requested review from a team and pfitzseb March 14, 2025 04:58
@semgrep-code-juliacomputing-new

Semgrep found 1 no-implicit-return finding:

Block-form functions should explicity return a value. If no meaningful
value is returned, explicity write return nothing instead.

Comment on lines +278 to +281
# We'll back up the old auth.toml though, because the user did not ask
# us to remove it, so we don't want to delete the token for them either.
# To avoid overwriting an existing backup, we generate a unique name
# by hashing the file contents.
Copy link
Member

Choose a reason for hiding this comment

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

We know the token is invalid though, so why do we want to keep it around?

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't want to delete user data if they didn't explicitly ask for it. And right now we delete iff force=true.

Co-authored-by: Sebastian Pfitzner <[email protected]>
@mortenpi mortenpi merged commit 9440d85 into main Mar 18, 2025
13 checks passed
@mortenpi mortenpi deleted the mp/auth-invalid-token branch March 18, 2025 22:12
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.

3 participants