Handle ci/cd job token #846
Conversation
lib/verify.js
Outdated
| switch(true) { | ||
| case str.startsWith("glcbt-"): | ||
| tokenHeader = "JOB-TOKEN" | ||
| default: | ||
| tokenHeader = "PRIVATE-TOKEN" | ||
| } |
There was a problem hiding this comment.
| switch(true) { | |
| case str.startsWith("glcbt-"): | |
| tokenHeader = "JOB-TOKEN" | |
| default: | |
| tokenHeader = "PRIVATE-TOKEN" | |
| } | |
| switch(gitlabToken) { | |
| case gitlabToken.startsWith("glcbt-"): | |
| tokenHeader = "JOB-TOKEN" | |
| default: | |
| tokenHeader = "PRIVATE-TOKEN" | |
| } |
@AlexandreEXFO - you need to switch the gitlabToken here.
There was a problem hiding this comment.
@toms-place: Yes, I'm a bit rusty on Javascript and have found several other places where the header should also be handled.
| switch(true) { | ||
| case token.startsWith("glcbt-"): | ||
| return "JOB-TOKEN" | ||
| default: | ||
| return "PRIVATE-TOKEN" | ||
| } |
There was a problem hiding this comment.
| switch(true) { | |
| case token.startsWith("glcbt-"): | |
| return "JOB-TOKEN" | |
| default: | |
| return "PRIVATE-TOKEN" | |
| } | |
| if (token.startsWith("glcbt-")) return "JOB-TOKEN"; | |
| return "PRIVATE-TOKEN"; |
I don't really understand the need to use switch(true) here?
There was a problem hiding this comment.
Gitlab have many types of tokens that could start with different prefixes (https://docs.gitlab.com/security/tokens/#token-prefixes). It's just to make it's easier to return the correct http header if any change is required in the future. I don't mind to use a "if" statement.
There was a problem hiding this comment.
GitLab supports configuring custom token prefixes for self-hosted instances. If used this would break the logic here.
There was a problem hiding this comment.
@fgreinacher: It's more complicated to support... We would have to add configuration parameters to support this. Currently, there is no regression with the change since if the prefix has been modified, it will continue to fall into the default case with it's the current implementation. It might be necessary to add a note in the documentation that currently CI job tokens with custom prefixes are not supported. I hadn't really planned on making the changes to support custom prefixes.
|
@travi: Would you have time to review this MR? Thanks in advance! |
|
Even if the job token can be used to push to the repository it would still fail for the other API calls, right? |
|
@fgreinacher: It's possible that there will be failures when calling other methods. It will depend on the CI job token's access to call these methods, but since the majority of calls except for push are read-only, I don't anticipate any. I haven't been able to test the modifications yet since the push feature with CI job token is still under development on Gitlab's side. |
IIUC the linked MRs are just about allowing Git push operations with the job token. We still won't be able to use it for the API calls which are needed for this plugin to work. |
|
@fgreinacher : It's already possible to call multiple APIs with a CI Job Token. This is something we do regularly in our CI/CD pipelines (e.g: upload/download something into the package registry). However, the HTTP header for passing the CI Job Token is not the same as a personal access token, which is why we're proposing the current change. I haven't done a thorough analysis of all the API calls. When the new Gitlab feature is added, I'll be able to test it to make sure it works end-to-end. |
|
@fgreinacher : I just tried 'curl --fail --header "JOB-TOKEN: $CI_JOB_TOKEN" "https://gitlab.com/api/v4/projects/"' and it doesn't work from a pipeline. :( |
|
Thanks for testing! We really need this to work with all the endpoints we're using to avoid frustration for users. |
|
I have set this to Draft state. We can revive later if/when GitLab supports the job token for all the required endpoints. |
Handle ci/cd job token to allow the use of the Gitlab CI/CD token having push rights in the repository (incoming feature)
Related to #156