Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ GITHUB_REQUEST_MULTIPLIER | The backoff multiplier for GitHub API requests.
GITHUB_REQUEST_RETRY_404 | Whether to retry GitHub API requests that return a 404 Not Found status. Defaults to true.
GITHUB_REQUEST_RETRY_422 | Whether to retry GitHub API requests that return a 422 Unprocessable Entity status. Defaults to true.
ENFORCE_READ_ONLY | Whether to enforce read-only permissions for all minted tokens. Defaults to false.
ENTERPRISE_CONFIG_ORGS | Comma-separated list of organization names authorized to configure enterprise scopes. Defaults to empty (disabled).


### CLI Usage
Expand Down Expand Up @@ -127,6 +128,7 @@ This command starts the GitHub Token Minter server.
| `--github-request-retry-404` | `GITHUB_REQUEST_RETRY_404` | Whether to retry GitHub API requests that return a 404 Not Found status. Defaults to true. |
| `--github-request-retry-422` | `GITHUB_REQUEST_RETRY_422` | Whether to retry GitHub API requests that return a 422 Unprocessable Entity status. Defaults to true. |
| `--enforce-read-only` | `ENFORCE_READ_ONLY` | Whether to enforce read-only permissions for all minted tokens. Defaults to false. |
| `--enterprise-config-orgs` | `ENTERPRISE_CONFIG_ORGS` | Comma-separated list of organization names authorized to configure enterprise scopes. Defaults to empty (disabled). |

#### `minty tools validate-cfg`

Expand Down
28 changes: 26 additions & 2 deletions pkg/config/config_evaluator.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,13 +43,17 @@ type ConfigEvaluator interface {
// was being evaluated when an error occurred. This could be a local file path or a remote
// system such as GitHub.
Eval(ctx context.Context, org, repo, scope string, token interface{}) (*Scope, string, error)
// EvalEnterprise looks for an enterprise configuration across the allowlisted organizations
// that contains the requested scope.
EvalEnterprise(ctx context.Context, scope string, token interface{}) (*Scope, string, error)
}

type configEvaluator struct {
loaders []ConfigFileLoader
loaders []ConfigFileLoader
enterpriseConfigOrgs []string
}

func NewConfigEvaluator(expireAt time.Duration, localConfigDir, repoConfigPath, orgConfigRepo, orgConfigPath, ref string, sourceSystem source.System) (*configEvaluator, error) {
func NewConfigEvaluator(expireAt time.Duration, localConfigDir, repoConfigPath, orgConfigRepo, orgConfigPath, ref string, sourceSystem source.System, enterpriseConfigOrgs []string) (*configEvaluator, error) {
// create an environment to compile any cel expressions
env, err := cel.NewEnv(
cel.Variable(AssertionKey, cel.DynType),
Expand Down Expand Up @@ -82,6 +86,7 @@ func NewConfigEvaluator(expireAt time.Duration, localConfigDir, repoConfigPath,
inRepoLoader,
orgLoader,
},
enterpriseConfigOrgs: enterpriseConfigOrgs,
}, nil
}

Expand Down Expand Up @@ -117,3 +122,22 @@ func (l *configEvaluator) Eval(ctx context.Context, org, repo, scope string, tok
}
return nil, fmt.Sprintf("%s/%s", org, repo), fmt.Errorf("error reading configuration, exhausted all possible source locations, failed to locate scope [%s] for repository [%s/%s].\nEvaluation results:\n%s", scope, org, repo, strings.Join(failureReasons, "\n"))
}

func (l *configEvaluator) EvalEnterprise(ctx context.Context, scope string, token interface{}) (*Scope, string, error) {

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This logic should be unit tested

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Antigravity: Done. Added dedicated unit tests TestConfigEvaluator_EvalEnterprise in pkg/config/config_evaluator_test.go covering disabled state, multi-org fallback, and all-org failure scenarios.

if len(l.enterpriseConfigOrgs) == 0 {
return nil, "", fmt.Errorf("enterprise token minting is not enabled: no organizations are authorized to configure enterprise scopes")
}

var failureReasons []string
for _, org := range l.enterpriseConfigOrgs {
s, source, err := l.Eval(ctx, org, "", scope, token)
if err == nil && s != nil {
return s, source, nil
}
if err != nil {
failureReasons = append(failureReasons, fmt.Sprintf("[%s]: %v", org, err))
}
}

return nil, "", fmt.Errorf("no permissions available for enterprise scope %q in allowlisted organizations. Evaluation errors:\n%s", scope, strings.Join(failureReasons, "\n"))
}
97 changes: 97 additions & 0 deletions pkg/config/config_evaluator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -388,3 +388,100 @@ func TestOrderedConfigFileLoader(t *testing.T) {
})
}
}

func TestConfigEvaluator_EvalEnterprise(t *testing.T) {
t.Parallel()

env, err := cel.NewEnv(cel.Variable(AssertionKey, cel.DynType))
if err != nil {
t.Fatal(err)
}

org1Loader := &compilingConfigLoader{
env: env,
loader: &testConfigFileLoader{
result: &Config{
Scopes: map[string]*Scope{
"ent_scope": {Rule: &Rule{If: "assertion.target == 'org1_val'"}},
},
},
},
}
org2Loader := &compilingConfigLoader{
env: env,
loader: &testConfigFileLoader{
result: &Config{
Scopes: map[string]*Scope{
"ent_scope": {Rule: &Rule{If: "assertion.target == 'org2_val'"}},
},
},
},
}

evaluator := &configEvaluator{
loaders: []ConfigFileLoader{
org1Loader,
org2Loader,
},
enterpriseConfigOrgs: []string{"org1", "org2"},
}

disabledEvaluator := &configEvaluator{
loaders: []ConfigFileLoader{
org1Loader,
},
}

cases := []struct {
name string
reader ConfigEvaluator
scope string
token interface{}
want *Scope
expErrMsg string
}{
{
name: "disabled_enterprise_minting",
reader: disabledEvaluator,
scope: "ent_scope",
token: map[string]string{"target": "org1_val"},
want: nil,
expErrMsg: "enterprise token minting is not enabled",
},
{
name: "fallback_to_second_allowlisted_org",
reader: evaluator,
scope: "ent_scope",
token: map[string]string{"target": "org2_val"},
want: &Scope{
Rule: &Rule{If: "assertion.target == 'org2_val'"},
},
expErrMsg: "",
},
{
name: "all_allowlisted_orgs_fail",
reader: evaluator,
scope: "ent_scope",
token: map[string]string{"target": "no_match"},
want: nil,
expErrMsg: "no permissions available for enterprise scope",
},
}

for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
ctx := t.Context()

got, _, err := tc.reader.EvalEnterprise(ctx, tc.scope, tc.token)
if diff := cmp.Diff(tc.want, got, cmp.FilterPath(func(p cmp.Path) bool {
return p.Last().String() != "Program"
}, cmp.Ignore())); diff != "" {
t.Errorf("mismatch (-want, +got):\n%s", diff)
}
if msg := testutil.DiffErrString(err, tc.expErrMsg); msg != "" {
t.Fatal(msg)
}
})
}
}
4 changes: 4 additions & 0 deletions pkg/config/config_loader_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -407,6 +407,10 @@ func (s *testSourceSystem) MintAccessToken(ctx context.Context, org, repo string
return "token", nil
}

func (s *testSourceSystem) MintEnterpriseAccessToken(ctx context.Context, enterprise string, permissions map[string]string) (string, error) {
return "token", nil
}

func (s *testSourceSystem) RetrieveFileContents(ctx context.Context, org, repo, filePath, ref string) ([]byte, error) {
return []byte{}, nil
}
Expand Down
8 changes: 8 additions & 0 deletions pkg/server/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ type Config struct {
GitHubRequestRetry404 bool
GitHubRequestRetry422 bool
EnforceReadOnly bool
EnterpriseConfigOrgs []string
}

// LogValue implements slog.LogValuer and returns a grouped value
Expand Down Expand Up @@ -240,6 +241,13 @@ func (cfg *Config) ToFlags(set *cli.FlagSet) *cli.FlagSet {
Default: []string{config.GitHubIssuer, config.GoogleIssuer},
})

f.StringSliceVar(&cli.StringSliceVar{
Comment thread
pdewilde marked this conversation as resolved.
Name: "enterprise-config-orgs",
Target: &cfg.EnterpriseConfigOrgs,
EnvVar: "ENTERPRISE_CONFIG_ORGS",
Usage: `The list of organizations authorized to define enterprise-scoped token permissions. If empty, enterprise-scoped tokens cannot be minted from organization configuration files.`,
})

f.Uint64Var(&cli.Uint64Var{
Name: "github-request-max-retries",
Target: &cfg.GitHubRequestMaxRetries,
Expand Down
1 change: 1 addition & 0 deletions pkg/server/runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ func Run(ctx context.Context, cfg *Config) error {
cfg.OrgConfigPath,
cfg.Ref,
sourceSystem,
cfg.EnterpriseConfigOrgs,
)
if err != nil {
return fmt.Errorf("failed to create config evaluator: %w", err)
Expand Down
69 changes: 69 additions & 0 deletions pkg/server/source/github.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
"fmt"
"io"
"net/http"
"net/url"
"strings"
"time"

Expand Down Expand Up @@ -149,6 +150,74 @@
return accessToken, nil
}

func (g *gitHubSourceSystem) installationForEnterprise(ctx context.Context, app *githubauth.App, enterprise string) (*githubauth.AppInstallation, error) {
jwtToken, err := app.AppToken()
if err != nil {
return nil, fmt.Errorf("failed to generate app JWT: %w", err)
}

client := github.NewClient(g.httpClient).WithAuthToken(jwtToken)
if g.baseURL != "" {
parsedURL, err := url.Parse(g.baseURL)
if err != nil {
return nil, fmt.Errorf("failed to parse base URL: %w", err)
}
if !strings.HasSuffix(parsedURL.Path, "/") {
parsedURL.Path += "/"
}
client.BaseURL = parsedURL
}

req, err := client.NewRequest(http.MethodGet, fmt.Sprintf("enterprises/%s/installation", enterprise), nil)
Comment thread
pdewilde marked this conversation as resolved.
if err != nil {
return nil, fmt.Errorf("failed to create request: %w", err)
}

var result github.Installation
_, err = client.Do(ctx, req, &result)
if err != nil {
return nil, fmt.Errorf("failed to get enterprise installation: %w", err)
}

return app.InstallationForID(ctx, fmt.Sprint(result.GetID()))

Check failure on line 182 in pkg/server/source/github.go

View workflow job for this annotation

GitHub Actions / lint (go|.)

error returned from external package is unwrapped: sig: func (*github.com/abcxyz/pkg/githubauth.App).InstallationForID(ctx context.Context, installationID string) (*github.com/abcxyz/pkg/githubauth.AppInstallation, error) (wrapcheck)
}

// MintEnterpriseAccessToken implements SourceSystem.
func (g *gitHubSourceSystem) MintEnterpriseAccessToken(ctx context.Context, enterprise string, permissions map[string]string) (string, error) {
var errs []error
var installation *githubauth.AppInstallation
var err error

logger := logging.FromContext(ctx)
for _, app := range g.apps {
installation, err = g.installationForEnterprise(ctx, app, enterprise)
if err != nil {
errs = append(errs, err)
} else {
break
}
}
if len(errs) != 0 {
return "", fmt.Errorf("errors retrieving GitHub enterprise installation: %w", errors.Join(errs...))
}

allRepoRequest := &githubauth.TokenRequestAllRepos{Permissions: map[string]string{}}

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this the right type of token?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Antigravity: Yes, AccessTokenAllRepos sends POST /app/installations/{installation_id}/access_tokens with {"permissions": ...} without specifying a repository array, which is the required GitHub payload for an Enterprise App installation access token.

if permissions != nil {
allRepoRequest.Permissions = permissions
}

logger.InfoContext(ctx, "sending request for enterprise to GitHub", "request", allRepoRequest)
accessToken, err := installation.AccessTokenAllRepos(ctx, allRepoRequest)
if err != nil {
if strings.Contains(err.Error(), "invalid http response status (expected 404 to be 201):") || strings.Contains(err.Error(), "invalid http response status (expected 422 to be 201):") {
logger.WarnContext(ctx, "error generating GitHub enterprise access token", "error", err)
return "", nil
}
return "", fmt.Errorf("error generating GitHub enterprise access token: %w", err)
}
return accessToken, nil
}

// RetrieveFileContents implements SourceSystem.
func (g *gitHubSourceSystem) RetrieveFileContents(ctx context.Context, org, repo, filePath, ref string) ([]byte, error) {
logger := logging.FromContext(ctx)
Expand Down
4 changes: 4 additions & 0 deletions pkg/server/source/source_system.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,10 @@ type System interface {
// the specified permissions.
MintAccessToken(ctx context.Context, org, repo string, repositories []string, permissions map[string]string) (string, error)

// MintEnterpriseAccessToken generates a new access token on behalf of the enterprise with
// the specified permissions.
MintEnterpriseAccessToken(ctx context.Context, enterprise string, permissions map[string]string) (string, error)

// RetrieveFileContents gets the contents of the file at filePath with the specified ref
// from the org/repo.
RetrieveFileContents(ctx context.Context, org, repo, filePath, ref string) ([]byte, error)
Expand Down
54 changes: 54 additions & 0 deletions pkg/server/token_minter.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ type TokenMinterServer struct {
// installation access token.
type TokenRequest struct {
OrgName string `json:"org_name"`
Enterprise string `json:"enterprise,omitempty"`
Repositories []string `json:"repositories"`
Permissions map[string]string `json:"permissions"`
Scope string `json:"scope"`
Expand Down Expand Up @@ -156,6 +157,59 @@ func (s *TokenMinterServer) processRequest(r *http.Request) *apiResponse {
return apiError
}

// If enterprise is requested, mint an enterprise-scoped access token directly.
if request.Enterprise != "" {
if request.OrgName != "" || len(request.Repositories) > 0 {
return &apiResponse{
http.StatusBadRequest,
"request for 'enterprise' also contained request for 'org_name' or 'repositories' and that is not allowed",
errors.New("request for 'enterprise' also contained request for 'org_name' or 'repositories' and that is not allowed"),
}
}
scope, _, err := s.configStore.EvalEnterprise(ctx, request.Scope, claims.asMap())
if err != nil {
if strings.Contains(err.Error(), "not enabled") {
return &apiResponse{
http.StatusForbidden,
err.Error(),
err,
}
}
return &apiResponse{
http.StatusForbidden,
fmt.Sprintf("requested scope %q is not found", request.Scope),
fmt.Errorf("error evaluating enterprise scope: %w", err),
}
}
if len(request.Permissions) == 0 {
if _, ok := scope.Permissions["*"]; ok {
request.Permissions = nil
} else {
request.Permissions = scope.Permissions
}
}
if err = validatePermissions(scope.Permissions, request.Permissions); err != nil {
Comment thread
pdewilde marked this conversation as resolved.
return &apiResponse{http.StatusForbidden, "requested permissions are not authorized for this scope", err}
}
if s.enforceReadOnly {

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if this is relevant for enterprise scoped tokens

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Antigravity: This logic handles the global --enforce-read-only flag to ensure any minted token (including enterprise tokens) respects read-only mode if enabled globally.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My point is there are a lot of claim scopes besides those two which can mutate. This is also relevant outside of enterprise scoped tokens as well.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Antigravity: Done. Replaced hardcoded contents/metadata map with enforceReadOnlyPermissions, which dynamically downgrades any requested/scope permission to "read" rather than stripping other permission keys.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets undo that for now. I misunderstood the existing state and thought it was only downgrading those two, but it instead just replaces the full permisison list with those two. I'm still not sure it makes sense for enterprise scope, maybe we should just throw an error if that read only permissions flag is set, I don't want to possibly escalate the permisssion we allow right now compared to existing state, as there may be a good reason we do it the way we do.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Antigravity: Done. Reverted standard org read-only behavior back to its original contents/metadata map, and updated enterprise token handling to explicitly return HTTP 403 Forbidden ("read-only mode is not supported for enterprise scoped tokens") when --enforce-read-only is active.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Antigravity: Done. Updated the error response for enterprise token requests in read-only mode to return HTTP 400 Bad Request.

return &apiResponse{
http.StatusBadRequest,
"read-only mode is not supported for enterprise scoped tokens",
errors.New("read-only mode is not supported for enterprise scoped tokens"),
}
}
logger.InfoContext(ctx, "generating enterprise token",
"claims", claims,
"enterprise", request.Enterprise,
"request_permissions", request.Permissions,
)
accessToken, err := s.sourceSystem.MintEnterpriseAccessToken(ctx, request.Enterprise, request.Permissions)
if err != nil {
return &apiResponse{http.StatusInternalServerError, "error generating enterprise access token", fmt.Errorf("error generating enterprise access token: %w", err)}
}
return &apiResponse{http.StatusOK, accessToken, nil}
}

// Determine the org name to be used for the request
requestOrgName, apiError := validateOrgName(request.OrgName, claims.ParsedOrgName)
if apiError != nil {
Expand Down
Loading
Loading