Support OIDC for sovereign clouds #321
Conversation
MoChilia
had a problem deploying
to
Automation test
GitHub Actions
Failure
MoChilia
had a problem deploying
to
Automation test
GitHub Actions
Failure
MoChilia
had a problem deploying
to
Automation test
GitHub Actions
Failure
MoChilia
had a problem deploying
to
Automation test
GitHub Actions
Failure
MoChilia
had a problem deploying
to
Automation test
GitHub Actions
Failure
|
Please follow the GitHub convention for Linking a pull request to an issue. |
|
#298 claims:
Please provide explanations in the PR description for
|
| core.error(`Please make sure to give write permissions to id-token in the workflow.`); | ||
| throw error; | ||
| } | ||
| if (!!federatedToken) { |
There was a problem hiding this comment.
Under what circumstances can federatedToken be null? Won't a null federatedToken trigger an error which can be caught by L118?
| let audience = core.getInput('audience', { required: false }); | ||
| //generating ID-token | ||
| let audience = core.getInput('audience', { required: false }); | ||
| try{ |
There was a problem hiding this comment.
Code style is important.
| try{ | |
| try { |
Read more from https://www.bing.com/search?q=why+is+code+style+important
| let [issuer, subjectClaim] = await jwtParser(federatedToken); | ||
| console.log("Federated token details: \n issuer - " + issuer + " \n subject claim - " + subjectClaim); | ||
| } | ||
| else{ |
There was a problem hiding this comment.
Same here
| else{ | |
| else { |
| } | ||
| catch (error) { | ||
| core.error(`${error.message.split(':')[1]}. Please make sure to give write permissions to id-token in the workflow.`); |
There was a problem hiding this comment.
All string operations should be accompanied by an example. Otherwise, it will be difficult for future readers to understand.
| } | ||
| if (!!federatedToken) { | ||
| let [issuer, subjectClaim] = await jwtParser(federatedToken); | ||
| console.log("Federated token details: \n issuer - " + issuer + " \n subject claim - " + subjectClaim); |
There was a problem hiding this comment.
Trailing spaces before an \n should be removed:
| console.log("Federated token details: \n issuer - " + issuer + " \n subject claim - " + subjectClaim); | |
| console.log("Federated token details:\n issuer - " + issuer + "\n subject claim - " + subjectClaim); |
Description
This pr is going to support OIDC based authentication for sovereign clouds.
The error in #298 was related to the wrong error handling in
login/src/main.ts
Line 125 in 4e65758
This line was expected to capture the error in the format like
Error message: xxx. However, when we used sovereign clouds then the error was thrown inlogin/src/main.ts
Line 119 in 4e65758
This error was not in the format like
Error message: xxx. Hence, when this error was caught, it would output the error message as mentioned in #298In addition, the previous code didn't throw the error and exit the process, instead, it just output the error message and continued the process. That's why we didn't expect customers to login successfully with OIDC for sovereign clouds in the previous version, but it still worked.
In this pr, we adjust the method of error handling. When the federated token is not fetched correctly, we would throw the error and exit the process. And we no longer limit customers to use OIDC for sovereign clouds. As long as the federated token is obtained successfully, we will attempt to login.
Test workflows