Skip to content

Resolve auth for Git providers that use tokens#6212

Merged
pditommaso merged 5 commits intomasterfrom
account-for-token-credentials
Jul 24, 2025
Merged

Resolve auth for Git providers that use tokens#6212
pditommaso merged 5 commits intomasterfrom
account-for-token-credentials

Conversation

@tcrespog
Copy link
Collaborator

@tcrespog tcrespog commented Jun 24, 2025

Those Git providers that rely on tokens for authentication (GitLab, Gitea, Bitbucket) do not need username and password to be defined too.

This PR tweaks the logic that determines whether a Git provider has authentication properties configured or not to take into account the sole presence of the token field.

@tcrespog tcrespog self-assigned this Jun 24, 2025
@netlify
Copy link

netlify bot commented Jun 24, 2025 •
edited
Loading

✅ Deploy Preview for nextflow-docs-staging canceled.

Name Link
🔨 Latest commit bdf2077
🔍 Latest deploy log https://app.netlify.com/projects/nextflow-docs-staging/deploys/6878e358d09dbc00081a74ac

@tcrespog tcrespog requested a review from pditommaso June 24, 2025 08:47
@pditommaso
Copy link
Member

pditommaso commented Jun 27, 2025 •
edited
Loading

It may be better to override it in the class that has support for it. I only see it supported by these

nextflow/modules/nextflow/src/main/groovy/nextflow/scm/GiteaRepositoryProvider.groovy

Lines 45 to 55 in b334df1

@Override
protected String[] getAuth() {
if( config.token ) {
// set the token in the request header
// https://docs.gitea.io/en-us/api-usage/#authentication
return new String[] { "Authorization", "token $config.token" as String }
}
else {
return EMPTY_ARRAY
}
}

nextflow/modules/nextflow/src/main/groovy/nextflow/scm/GitlabRepositoryProvider.groovy

Lines 42 to 51 in b334df1

protected String[] getAuth() {
if( config.token ) {
// set the token in the request header
return new String[] { "PRIVATE-TOKEN", config.token }
}
if( config.password ) {
return new String[] { "PRIVATE-TOKEN", config.password }
}
return EMPTY_ARRAY
}

@tcrespog
Copy link
Collaborator Author

tcrespog commented Jun 27, 2025

It may be better to override it in the class that has support for it. I only see it supported by these

The idea of adding it in the parent class is that more may come, such as #6209.

@pditommaso
Copy link
Member

pditommaso commented Jul 6, 2025

I assume this is addressed by #6209. @tcrespog let me know if that's not the case

@pditommaso pditommaso closed this Jul 6, 2025
@tcrespog
Copy link
Collaborator Author

tcrespog commented Jul 16, 2025

I assume this is addressed by #6209. @tcrespog let me know if that's not the case

Not exactly, providers other than Bitbucket (especially GitLab) would need the logic implemented in this PR too. I'm re-opening the PR; do you want me to address this comment or is it okay as it is?

@tcrespog tcrespog reopened this Jul 16, 2025
@pditommaso
Copy link
Member

pditommaso commented Jul 16, 2025

I see. Yes, it would be better to handle at specific provider level

@pditommaso
Copy link
Member

pditommaso commented Jul 17, 2025

worth adding a unit test?

@pditommaso pditommaso merged commit 9d8943e into master Jul 24, 2025
19 of 22 checks passed
@pditommaso pditommaso deleted the account-for-token-credentials branch July 24, 2025 10:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pditommaso pditommaso pditommaso approved these changes

Assignees

@tcrespog tcrespog

Labels

software/git

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@tcrespog @pditommaso @bentsherman