mirror of
https://codeberg.org/forgejo/forgejo.git
synced 2026-05-12 22:10:25 +00:00
feat: allow Authorized Integrations to have multiple values for a claim match (#12482)
Adds new Authorized Integration claim comparison rules for "in a list" and "in a list of globs", which would be required to permit multiple Forgejo Action events to match a JWT (per [design work](https://codeberg.org/forgejo/forgejo/issues/3571#issuecomment-14510514), [comment](https://codeberg.org/forgejo/forgejo/issues/3571#issuecomment-14512185)). ## Checklist The [contributor guide](https://forgejo.org/docs/next/contributor/) contains information that will be helpful to first time contributors. All work and communication must conform to Forgejo's [AI Agreement](https://codeberg.org/forgejo/governance/src/branch/main/AIAgreement.md). There also are a few [conditions for merging Pull Requests in Forgejo repositories](https://codeberg.org/forgejo/governance/src/branch/main/PullRequestsAgreement.md). You are also welcome to join the [Forgejo development chatroom](https://matrix.to/#/#forgejo-development:matrix.org). ### Tests for Go changes - I added test coverage for Go changes... - [x] in their respective `*_test.go` for unit tests. - [ ] in the `tests/integration` directory if it involves interactions with a live Forgejo server. - I ran... - [x] `make pr-go` before pushing ### Documentation - [ ] I created a pull request [to the documentation](https://codeberg.org/forgejo/docs) to explain to Forgejo users how to use this change. - [x] I did not document these changes and I do not expect someone else to do it. ### Release notes - [ ] This change will be noticed by a Forgejo user or admin (feature, bug fix, performance, etc.). I suggest to include a release note for this change. - [x] This change is not visible to a Forgejo user or admin (refactor, dependency upgrade, etc.). I think there is no need to add a release note for this change. Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/12482 Reviewed-by: Gusted <gusted@noreply.codeberg.org>
This commit is contained in:
parent
a4d623148d
commit
e5eb5f8e63
4 changed files with 152 additions and 4 deletions
|
|
@ -31,6 +31,12 @@ user-defined rules, and grant access to Forgejo's API on behalf of a user.
|
|||
The issuer may be set to "urn:forgejo:authorized-integrations:actions"
|
||||
to support JWTs from the local instance's Forgejo Actions, utilizing the
|
||||
enable-openid-connect flag in a workflow.`,
|
||||
|
||||
// `--claim-in sub=v1,v2,v3` needs to be parsed as a single parameter so that we can comma-split the value into
|
||||
// an array. To accomplish this, we disable urfave 's slice flag separator, which would cause this to be
|
||||
// treated as "sub=v1", "v2=?", and "v3=?", resulting in an error of missing values.
|
||||
DisableSliceFlagSeparator: true,
|
||||
|
||||
Flags: []cli.Flag{
|
||||
&cli.StringFlag{
|
||||
Name: "username",
|
||||
|
|
@ -59,11 +65,21 @@ enable-openid-connect flag in a workflow.`,
|
|||
Value: map[string]string{},
|
||||
Usage: `Zero-or-more claim equality checks, formatted as claim=value, example: "actor=someuser"`,
|
||||
},
|
||||
&cli.StringMapFlag{
|
||||
Name: "claim-in",
|
||||
Value: map[string]string{},
|
||||
Usage: `Zero-or-more claim equality in list checks, formatted as claim=value1,value2,... example: "actor=user1,user2"`,
|
||||
},
|
||||
&cli.StringMapFlag{
|
||||
Name: "claim-glob",
|
||||
Value: map[string]string{},
|
||||
Usage: `Zero-or-more claim glob checks, formatted as claim=value, example: "sub=repo:forgejo/*:pull_request"`,
|
||||
},
|
||||
&cli.StringMapFlag{
|
||||
Name: "claim-glob-in",
|
||||
Value: map[string]string{},
|
||||
Usage: `Zero-or-more claim glob in list checks, formatted as claim=va*ue1,va*ue2,... example: "sub=repo:*/*:pull_request,repo:*/*:refs:*"`,
|
||||
},
|
||||
// nested claim support omitted for now -- pretty complex for a CLI
|
||||
|
||||
// Permissions available on successful auth:
|
||||
|
|
@ -115,6 +131,17 @@ func runCreateAuthorizedIntegration(ctx context.Context, c *cli.Command) error {
|
|||
Value: value,
|
||||
})
|
||||
}
|
||||
for claim, value := range c.StringMap("claim-in") {
|
||||
values := []string{}
|
||||
for s := range strings.SplitSeq(value, ",") {
|
||||
values = append(values, strings.TrimSpace(s))
|
||||
}
|
||||
rules = append(rules, auth_model.ClaimRule{
|
||||
Claim: claim,
|
||||
Comparison: auth_model.ClaimIn,
|
||||
Values: values,
|
||||
})
|
||||
}
|
||||
for claim, value := range c.StringMap("claim-glob") {
|
||||
rules = append(rules, auth_model.ClaimRule{
|
||||
Claim: claim,
|
||||
|
|
@ -122,6 +149,17 @@ func runCreateAuthorizedIntegration(ctx context.Context, c *cli.Command) error {
|
|||
Value: value,
|
||||
})
|
||||
}
|
||||
for claim, value := range c.StringMap("claim-glob-in") {
|
||||
values := []string{}
|
||||
for s := range strings.SplitSeq(value, ",") {
|
||||
values = append(values, strings.TrimSpace(s))
|
||||
}
|
||||
rules = append(rules, auth_model.ClaimRule{
|
||||
Claim: claim,
|
||||
Comparison: auth_model.ClaimGlobIn,
|
||||
Values: values,
|
||||
})
|
||||
}
|
||||
ai.ClaimRules = &auth_model.ClaimRules{Rules: rules}
|
||||
|
||||
scopes := strings.Join(c.StringSlice("scope"), ",")
|
||||
|
|
@ -179,7 +217,8 @@ func runCreateAuthorizedIntegration(ctx context.Context, c *cli.Command) error {
|
|||
Description string `json:"description"`
|
||||
Claim string `json:"claim"`
|
||||
Comparison auth_model.ClaimComparison `json:"compare"`
|
||||
Value string `json:"value"`
|
||||
Value string `json:"value,omitempty"`
|
||||
Values []string `json:"values,omitempty"`
|
||||
}
|
||||
output := struct {
|
||||
Message string `json:"message"`
|
||||
|
|
@ -200,14 +239,19 @@ func runCreateAuthorizedIntegration(ctx context.Context, c *cli.Command) error {
|
|||
switch cr.Comparison {
|
||||
case auth_model.ClaimEqual:
|
||||
description = fmt.Sprintf("%q = %q", cr.Claim, cr.Value)
|
||||
case auth_model.ClaimIn:
|
||||
description = fmt.Sprintf("%q in %q", cr.Claim, cr.Values)
|
||||
case auth_model.ClaimGlob:
|
||||
description = fmt.Sprintf("%q matches %q", cr.Claim, cr.Value)
|
||||
case auth_model.ClaimGlobIn:
|
||||
description = fmt.Sprintf("%q matches in %q", cr.Claim, cr.Values)
|
||||
}
|
||||
output.ClaimRules = append(output.ClaimRules, ClaimRuleDescription{
|
||||
Description: description,
|
||||
Claim: cr.Claim,
|
||||
Comparison: cr.Comparison,
|
||||
Value: cr.Value,
|
||||
Values: cr.Values,
|
||||
})
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -111,6 +111,9 @@ type ClaimRule struct {
|
|||
// For Comparison of ClaimEqual or ClaimGlob, the specific value or glob to match against
|
||||
Value string `json:"value,omitempty"`
|
||||
|
||||
// For Comparison of ClaimIn or ClaimGlobIn, an array of values to match against
|
||||
Values []string `json:"values,omitempty"`
|
||||
|
||||
// For ClaimNested, the rules to apply to the nested object
|
||||
Nested *ClaimRules `json:"nested,omitempty"`
|
||||
}
|
||||
|
|
@ -119,7 +122,9 @@ type ClaimComparison string
|
|||
|
||||
const (
|
||||
ClaimEqual ClaimComparison = "eq" // exactly equal claim
|
||||
ClaimIn ClaimComparison = "in" // exactly equal any of the options in a list
|
||||
ClaimGlob ClaimComparison = "glob" // glob match complete claim string
|
||||
ClaimGlobIn ClaimComparison = "glob-in" // glob match any of the options in a list
|
||||
ClaimNested ClaimComparison = "nest" // recurse into a claim that is an map[string]any with it's own data fields
|
||||
)
|
||||
|
||||
|
|
|
|||
|
|
@ -460,6 +460,13 @@ func (a *AuthorizedIntegration) checkClaims(incomingClaims any, stored *auth_mod
|
|||
} else if lhsStr != rule.Value {
|
||||
return fmt.Errorf("claim %q must be %q, but was %q", rule.Claim, rule.Value, lhsStr)
|
||||
}
|
||||
case auth_model.ClaimIn:
|
||||
lhsStr, ok := lhs.(string)
|
||||
if !ok {
|
||||
return fmt.Errorf("claim %q must be a string, but was %T", rule.Claim, lhs)
|
||||
} else if !slices.Contains(rule.Values, lhsStr) {
|
||||
return fmt.Errorf("claim %q must be one of %q, but was %q", rule.Claim, rule.Values, lhsStr)
|
||||
}
|
||||
case auth_model.ClaimGlob:
|
||||
lhsStr, ok := lhs.(string)
|
||||
if !ok {
|
||||
|
|
@ -472,6 +479,25 @@ func (a *AuthorizedIntegration) checkClaims(incomingClaims any, stored *auth_mod
|
|||
if !r.Match(lhsStr) {
|
||||
return fmt.Errorf("claim %q must match glob %q, but value %q did not match", rule.Claim, rule.Value, lhsStr)
|
||||
}
|
||||
case auth_model.ClaimGlobIn:
|
||||
lhsStr, ok := lhs.(string)
|
||||
if !ok {
|
||||
return fmt.Errorf("claim %q must be a string, but was %T", rule.Claim, lhs)
|
||||
}
|
||||
matched := false
|
||||
for _, g := range rule.Values {
|
||||
r, err := glob.Compile(g)
|
||||
if err != nil {
|
||||
return fmt.Errorf("unable to parse glob for claim rule on %q; glob = %q, err = %w", rule.Claim, g, err)
|
||||
}
|
||||
if r.Match(lhsStr) {
|
||||
matched = true
|
||||
break
|
||||
}
|
||||
}
|
||||
if !matched {
|
||||
return fmt.Errorf("claim %q must glob match one of %q, but value %q did not match", rule.Claim, rule.Values, lhsStr)
|
||||
}
|
||||
case auth_model.ClaimNested:
|
||||
lhsMap, ok := lhs.(map[string]any)
|
||||
if !ok {
|
||||
|
|
|
|||
|
|
@ -42,6 +42,13 @@ func TestCheckClaims(t *testing.T) {
|
|||
Value: value,
|
||||
}
|
||||
}
|
||||
in := func(claim string, values []string) auth_model.ClaimRule {
|
||||
return auth_model.ClaimRule{
|
||||
Claim: claim,
|
||||
Comparison: auth_model.ClaimIn,
|
||||
Values: values,
|
||||
}
|
||||
}
|
||||
glob := func(claim, value string) auth_model.ClaimRule {
|
||||
return auth_model.ClaimRule{
|
||||
Claim: claim,
|
||||
|
|
@ -49,6 +56,13 @@ func TestCheckClaims(t *testing.T) {
|
|||
Value: value,
|
||||
}
|
||||
}
|
||||
globIn := func(claim string, values []string) auth_model.ClaimRule {
|
||||
return auth_model.ClaimRule{
|
||||
Claim: claim,
|
||||
Comparison: auth_model.ClaimGlobIn,
|
||||
Values: values,
|
||||
}
|
||||
}
|
||||
nest := func(claim string, inner ...auth_model.ClaimRule) auth_model.ClaimRule {
|
||||
return auth_model.ClaimRule{
|
||||
Claim: claim,
|
||||
|
|
@ -161,6 +175,32 @@ func TestCheckClaims(t *testing.T) {
|
|||
require.ErrorContains(t, ai.checkClaims(c, rules), "claim \"arbitrary\" must be a string, but was int")
|
||||
})
|
||||
|
||||
t.Run("comparison ClaimIn", func(t *testing.T) {
|
||||
c := map[string]any{}
|
||||
rules := rules(in("arbitrary", []string{"abc", "def"}))
|
||||
|
||||
c["arbitrary"] = "abc"
|
||||
require.NoError(t, ai.checkClaims(c, rules))
|
||||
c["arbitrary"] = "def"
|
||||
require.NoError(t, ai.checkClaims(c, rules))
|
||||
|
||||
c["arbitrary"] = "123"
|
||||
require.ErrorContains(t, ai.checkClaims(c, rules), "claim \"arbitrary\" must be one of [\"abc\" \"def\"], but was \"123\"")
|
||||
|
||||
c["arbitrary"] = 123
|
||||
require.ErrorContains(t, ai.checkClaims(c, rules), "claim \"arbitrary\" must be a string, but was int")
|
||||
})
|
||||
|
||||
t.Run("comparison ClaimIn empty", func(t *testing.T) {
|
||||
c := map[string]any{}
|
||||
rules := rules(in("arbitrary", []string{}))
|
||||
|
||||
require.ErrorContains(t, ai.checkClaims(c, rules), "claim rule on \"arbitrary\" couldn't be satisfied: claim not found")
|
||||
|
||||
c["arbitrary"] = "abc"
|
||||
require.ErrorContains(t, ai.checkClaims(c, rules), "claim \"arbitrary\" must be one of [], but was \"abc\"")
|
||||
})
|
||||
|
||||
t.Run("comparison ClaimGlob", func(t *testing.T) {
|
||||
c := map[string]any{}
|
||||
r := rules(glob("arbitrary", "*c"))
|
||||
|
|
@ -182,6 +222,39 @@ func TestCheckClaims(t *testing.T) {
|
|||
require.ErrorContains(t, ai.checkClaims(c, r), "unable to parse glob for claim rule on \"arbitrary\"; glob = \"[abc\", err = unexpected end of input")
|
||||
})
|
||||
|
||||
t.Run("comparison ClaimGlobIn", func(t *testing.T) {
|
||||
c := map[string]any{}
|
||||
r := rules(globIn("arbitrary", []string{"*c", "*def*"}))
|
||||
|
||||
c["arbitrary"] = "abc"
|
||||
require.NoError(t, ai.checkClaims(c, r))
|
||||
c["arbitrary"] = "abcdef"
|
||||
require.NoError(t, ai.checkClaims(c, r))
|
||||
|
||||
c["arbitrary"] = "123"
|
||||
require.ErrorContains(t, ai.checkClaims(c, r), "claim \"arbitrary\" must glob match one of [\"*c\" \"*def*\"], but value \"123\" did not match")
|
||||
|
||||
c["arbitrary"] = "this string contains a c or two but doesn't end with one" // ensure glob isn't OK w/ a partial match
|
||||
require.ErrorContains(t, ai.checkClaims(c, r), "claim \"arbitrary\" must glob match one of [\"*c\" \"*def*\"], but value \"this string contains a c or two but doesn't end with one\" did not match")
|
||||
|
||||
c["arbitrary"] = 123
|
||||
require.ErrorContains(t, ai.checkClaims(c, r), "claim \"arbitrary\" must be a string, but was int")
|
||||
|
||||
r = rules(globIn("arbitrary", []string{"[abc"}))
|
||||
c["arbitrary"] = "abc"
|
||||
require.ErrorContains(t, ai.checkClaims(c, r), "unable to parse glob for claim rule on \"arbitrary\"; glob = \"[abc\", err = unexpected end of input")
|
||||
})
|
||||
|
||||
t.Run("comparison ClaimGlobIn empty", func(t *testing.T) {
|
||||
c := map[string]any{}
|
||||
rules := rules(globIn("arbitrary", []string{}))
|
||||
|
||||
require.ErrorContains(t, ai.checkClaims(c, rules), "claim rule on \"arbitrary\" couldn't be satisfied: claim not found")
|
||||
|
||||
c["arbitrary"] = "abc"
|
||||
require.ErrorContains(t, ai.checkClaims(c, rules), "claim \"arbitrary\" must glob match one of [], but value \"abc\" did not match")
|
||||
})
|
||||
|
||||
t.Run("comparison ClaimNested", func(t *testing.T) {
|
||||
c := map[string]any{}
|
||||
r := rules(nest("nest", eq("arbitrary", "abc")))
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue