GitHub team auth #745
Merged
GitHub team auth #745
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This makes way for IsTeamMember
This allows empire to be configured to only allow access to members of a specific team within an organization.
Other messages are appended to these if an error occurs.
| @@ -108,5 +108,16 @@ func newAuthenticator(c *cli.Context, e *empire.Empire) auth.Authenticator { | |||
| ) | |||
| } | |||
|
|
|||
| // After the user is authenticated, check their GitHub Team membership. | |||
| if teamId := c.String(FlagGithubTeam); teamId != "" { | |||
There was a problem hiding this comment.
Minor, but can we change the casing from Id to ID. There's some doc from google about how it's not idiomatic to use camel casing for things like ID and we try to follow that.
|
Looks great 👍 |
| log.Println("Adding GitHub Team authorizer with the following configuration:") | ||
| log.Println(fmt.Sprintf(" Team ID: %v ", teamId)) | ||
|
|
||
| return auth.WithAuthorization(authenticator, authorizer) |
There was a problem hiding this comment.
Should we cache this authorization, like we do with the org auth? Without caching, every empire API hit incurs a full 1-2 round trips to GitHub, which makes everything slow. An in memory ~30 minute cache has worked pretty well.
There was a problem hiding this comment.
ah, i didn't realize this was on every API call, I thought it just happened once when the user logged in.
definitely makes sense to cache.
mwildehahn
force-pushed
the
github-team-auth
branch
from
815e061
to
7f06c32
Compare
It now handles fetching the authenticated user and passing the GitHub login to the teams endpoint. This way we don't rely on User.Name always being the GitHub login.
|
thanks for the review @ejholmes! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This allows users to limit empire access to members of a specific team within a github organization.
Github doesn't support fetching teams by name without a non trivial amount of work, so you have to use the team id.
To get the team id:
curl -i -u <username>:<password> https://api.github.com/orgs/<organization>/teamstesting: