-
Notifications
You must be signed in to change notification settings - Fork 3
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
Resolve SAML assertion issues #50
Conversation
@@ -52,7 +52,8 @@ exports.onExecutePostLogin = async (event, api) => { | |||
// Set SAML attribute for the user's GitHub team memberships | |||
const userTeamsResponse = await octokit.request('GET /user/teams').catch(error => api.access.deny(`Error retrieving teams from GitHub: ${error}`)) | |||
const userTeamSlugs = userTeamsResponse.data.map(team => team.slug) | |||
api.samlResponse.setAttribute('https://aws.amazon.com/SAML/Attributes/PrincipalTag:github_team', userTeamSlugs.join(',')) | |||
console.log("Joined the following teams:" `${userTeamSlugs.join(',')}`) |
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.
Logline needs updating to 👇
console.log("Joined the following teams:" `${userTeamSlugs.join(',')}`) | |
console.log(`Joined the following teams: ${userTeamSlugs.join(',')}`) |
Interpreter is getting confused about two separate strings being passed to the join function 🙈
Will also need to update the test on line 64 to 👇
- ['https://aws.amazon.com/SAML/Attributes/PrincipalTag:github_team', 'test-team-1,test-team-2'],
+ ['https://aws.amazon.com/SAML/Attributes/AccessControl:github_team', 'test-team-1,test-team-2'],
To reflect the new changes 🧪 🪞 😁
… and doesn't need to be defined in code
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.
🚀 Looks Good To Me!
This PR makes a few changes to how github teams are passed back as a SAML attribute to AWS.
PrincipalTag
toAccessControl
in line with this guidance.