-
Notifications
You must be signed in to change notification settings - Fork 734
New command group: nextflow auth
#6380
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
base: master
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for nextflow-docs-staging canceled.
|
e9b3f26
to
c01e56b
Compare
5e79a3b
to
c5a3283
Compare
Struggling to get the tests to pass. Even |
nextflow auth login
nextflow auth
@claude Please review this pull request and provide feedback on:
Use the repository's CLAUDE.md for guidance on style and conventions. Be constructive and helpful in your feedback. Use |
Claude encountered an error —— View job
I'll analyze this and get back to you. |
} else { | ||
// Reset only what was set | ||
if (bold) fmt = fmt.boldOff() | ||
if (dim) fmt = fmt.a(Attribute.INTENSITY_BOLD) // Reset dim |
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.
Bug: Colorize Method Incorrectly Applies Bold Formatting
When fullReset
is false, the colorize
method attempts to reset dim
formatting using Attribute.INTENSITY_BOLD
. This applies bold formatting instead of returning the text to normal intensity, causing text to appear bold when it should be undimmed.
// Check if TOWER_WORKFLOW_ID environment variable is set | ||
def envWorkspaceId = System.getenv('TOWER_WORKFLOW_ID') | ||
if (envWorkspaceId) { | ||
println "\nDefault workspace: ${ColorUtil.colorize('TOWER_WORKFLOW_ID environment variable is set', 'yellow')}" |
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.
Bug: Incorrect Environment Variable Check in Nextflow Auth
The nextflow auth
commands incorrectly check for the TOWER_WORKFLOW_ID
environment variable when configuring and displaying the default workspace. This prevents the system from recognizing the intended TOWER_WORKSPACE_ID
environment variable for workspace selection.
Additional Locations (1)
Let's start refactoring this as |
I get confused a bit on how -url is managed. I think it is better to simplify it by -url is for enterprise with PAT and no url is for cloud. Moreover, instead of hardcoding all environments, keep just the production values as default and allow to test in other environments using env variables. In fact, we already have the TOWER_API_ENDPOINT, we just would need to add the ones for auth endpoint and client id. What do you think about it? |
if (isCloudEndpoint(apiUrl)) { | ||
def tokenId = decodeTokenId(existingToken as String) | ||
deleteTokenViaApi(existingToken as String, apiUrl, tokenId) | ||
} else { | ||
println " - Enterprise installation detected - PAT will not be deleted from platform." | ||
} | ||
|
||
removeAuthFromConfig() | ||
|
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.
What happens if there was a previous token already defined in the config. Could it be deleted by mistake?
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.
Yes it will be deleted, by design. Maybe we could add a confirmation prompt?
@jorgee I think that we still need the logic to detect the 3 prod / stage / dev URLs for cloud - it's needed to trigger the logic to try the Auth0 flow rather than just booting people to use a PAT. If we have that logic in the code, then I don't really see a reason to not also include the key and auth0 URL personally, it's only a handful of extra lines of code. And it'd be a swap for env vars. Given that I don't envision ever needing to test with any other auth0 URLs I'm not really sure it'd be any simpler..? |
Signed-off-by: Phil Ewels <[email protected]>
Signed-off-by: Phil Ewels <[email protected]>
Signed-off-by: Phil Ewels <[email protected]>
Signed-off-by: Phil Ewels <[email protected]>
Signed-off-by: Phil Ewels <[email protected]>
Set TOWER_ACCESS_TOKEN with new PAT. Signed-off-by: Phil Ewels <[email protected]>
Signed-off-by: Phil Ewels <[email protected]>
Signed-off-by: Phil Ewels <[email protected]>
Signed-off-by: Phil Ewels <[email protected]>
Signed-off-by: Phil Ewels <[email protected]>
Signed-off-by: Phil Ewels <[email protected]>
Signed-off-by: Phil Ewels <[email protected]>
Signed-off-by: Phil Ewels <[email protected]>
Signed-off-by: Phil Ewels <[email protected]>
Signed-off-by: Phil Ewels <[email protected]>
Signed-off-by: Phil Ewels <[email protected]>
Signed-off-by: Phil Ewels <[email protected]>
Signed-off-by: Phil Ewels <[email protected]>
Signed-off-by: Phil Ewels <[email protected]>
Signed-off-by: Phil Ewels <[email protected]>
Signed-off-by: Phil Ewels <[email protected]>
Signed-off-by: Phil Ewels <[email protected]>
Signed-off-by: Phil Ewels <[email protected]>
Signed-off-by: Phil Ewels <[email protected]>
Signed-off-by: Phil Ewels <[email protected]>
Signed-off-by: Phil Ewels <[email protected]>
Signed-off-by: Phil Ewels <[email protected]>
Signed-off-by: Phil Ewels <[email protected]>
Signed-off-by: Phil Ewels <[email protected]>
Signed-off-by: jorgee <[email protected]>
Signed-off-by: jorgee <[email protected]>
It seems login is not working well when there is a profile with tower properties. I had already defined a profile and it was removed from the profile and wrote a new one. I will debug it, but I was wondering if it will be easier to write everything in a separate file and just add the include it in the .nextflow/config. We will just need to add or remove the line instead of looking for the tower config. |
Signed-off-by: jorgee <[email protected]>
24d0865
to
0273b40
Compare
Pushed changes to move Auth functionality to nf-tower |
Signed-off-by: jorgee <[email protected]>
apiUrl = normalizeApiUrl(apiUrl) | ||
ColorUtil.printColored(" - Seqera Platform API endpoint: ${ColorUtil.colorize(apiUrl, 'magenta')} (can be customised with ${ColorUtil.colorize('-url', 'cyan')})", "dim") | ||
|
||
// Check if this is a cloud endpoint or enterprise | ||
def endpointInfo = getCloudEndpointInfo(apiUrl) | ||
if( endpointInfo.isCloud ) { | ||
try { | ||
performAuth0Login(apiUrl, endpointInfo.environment as String) | ||
} catch( Exception e ) { | ||
log.debug("Authentication failed", e) | ||
println "" | ||
throw new AbortOperationException("${e.message}") | ||
} | ||
} else { | ||
// Enterprise endpoint - use PAT authentication | ||
handleEnterpriseAuth(apiUrl) | ||
} |
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.
This is what I referred to in my previous comment #6380 (comment).
In normalizeApi
, it is converting http to https and, in getCloudEndpoint
, it is checking if the provided apiUrl is in a legacy format '/api' to compare it with the dev, stage and prod endpoints
Assuming that url
flag is just for login in enterprise, it could be simplify by
if (apiUrl)
handleEnterpriseAuth(apiUrl)
else
performAuth0Login()
No need to normalizeApi
function nor getCloudEndpointInfo
. If the problem is providing the Auth0 url and clientId when testing we could be kept in a map api_endpoint -> auth0_Info and just use the TOWER_ENDPOINT_API to retrive them as it is used in the rest of the nf-tower calls.
This PR introduces a complete authentication system for Seqera Platform with multiple subcommands for managing authentication credentials and configuration.
CleanShot.2025-09-21.at.00.32.54.mp4
Features
New
nextflow auth
command with subcommands:nextflow auth login
- Authenticate with Seqera Platform using OAuth2/PKCE flownextflow auth logout
- Remove authentication credentials and clear configurationnextflow auth status
- Show current authentication status and user informationnextflow auth config
- Display current authentication configurationAuth0 Device Flow Authentication
Usage Examples
Technical Implementation
Core Components
Authentication Flow
Note
Introduces a new
nextflow auth
CLI with OAuth2 (Auth0) and PAT flows via nf-tower plugin, adds ANSI color utility, integrates into launcher, and includes comprehensive tests.CmdAuth
command group with subcommands:login
,logout
,config
,status
.auth
intoLauncher
command registry.ColorUtil
for ANSI-colored terminal output.io.seqera.tower.plugin.cli.AuthCommandImpl
(extension ofCmdAuth.AuthCommand
).nextflowVersion
to25.08.0-edge
inplugins/nf-tower/build.gradle
.CmdAuth
,ColorUtil
, andAuthCommandImpl
.nextflow/trace/ReportObserver.groovy
.Written by Cursor Bugbot for commit 00b4590. This will update automatically on new commits. Configure here.