Fix wrong secret name
in 'sub' claim of OIDC token
In the current implementation, in the `sub` claim value `secret:{zuul tenant}/{canonical project name}/{secret name}` the `{secret name}` is the Ansbile var name defined in `job.secrets.name` while it should be the `secret.name`. This change fixes this issue by switch to use `secret.name`. Change-Id: I9cccc5f14bb0ff17f389c14e89c35fb979ea5c49
This commit is contained in:
@ -1,5 +1,5 @@
|
||||
- secret:
|
||||
name: oidc_secret
|
||||
name: oidc-secret
|
||||
oidc:
|
||||
ttl: 300
|
||||
algorithm: RS256
|
||||
@ -10,7 +10,7 @@
|
||||
sub: some_sub_value
|
||||
|
||||
- secret:
|
||||
name: oidc_secret_iss_override_allowed
|
||||
name: oidc-secret-iss-override-allowed
|
||||
oidc:
|
||||
ttl: 300
|
||||
algorithm: RS256
|
||||
@ -26,8 +26,10 @@
|
||||
name: project2-oidc-secret
|
||||
run: playbooks/secret.yaml
|
||||
secrets:
|
||||
- oidc_secret
|
||||
- oidc_secret_iss_override_allowed
|
||||
- name: oidc_secret
|
||||
secret: oidc-secret
|
||||
- name: oidc_secret_iss_override_allowed
|
||||
secret: oidc-secret-iss-override-allowed
|
||||
|
||||
- project:
|
||||
check:
|
||||
|
@ -14,7 +14,7 @@
|
||||
+150AKGNZpPJnnP3QYY4W/MWcKH/zdO400+zWN52WevbSqZy90tqKDJrBkMl1ydqbuw1E4ZHvIs=
|
||||
|
||||
- secret:
|
||||
name: project2_oidc_secret0
|
||||
name: project2-oidc-secret0
|
||||
oidc:
|
||||
ttl: 300
|
||||
algorithm: RS256
|
||||
@ -25,7 +25,7 @@
|
||||
sub: some_sub_value
|
||||
|
||||
- secret:
|
||||
name: project2_oidc_secret1
|
||||
name: project2-oidc-secret1
|
||||
oidc:
|
||||
ttl: 300
|
||||
algorithm: RS256
|
||||
@ -36,7 +36,7 @@
|
||||
sub: some_sub_value
|
||||
|
||||
- secret:
|
||||
name: project2_oidc_secret2
|
||||
name: project2-oidc-secret2
|
||||
oidc:
|
||||
ttl: 300
|
||||
algorithm: RS256
|
||||
@ -52,9 +52,12 @@
|
||||
run: playbooks/secret.yaml
|
||||
secrets:
|
||||
- project2_secret
|
||||
- project2_oidc_secret0
|
||||
- project2_oidc_secret1
|
||||
- project2_oidc_secret2
|
||||
- name: project2_oidc_secret0
|
||||
secret: project2-oidc-secret0
|
||||
- name: project2_oidc_secret1
|
||||
secret: project2-oidc-secret1
|
||||
- name: project2_oidc_secret2
|
||||
secret: project2-oidc-secret2
|
||||
|
||||
- project:
|
||||
check:
|
||||
|
@ -1,5 +1,5 @@
|
||||
- secret:
|
||||
name: project2_oidc_secret
|
||||
name: project2-oidc-secret
|
||||
oidc:
|
||||
ttl: 300
|
||||
algorithm: RS256
|
||||
@ -14,7 +14,8 @@
|
||||
name: project2-oidc-secret
|
||||
run: playbooks/secret.yaml
|
||||
secrets:
|
||||
- project2_oidc_secret
|
||||
- name: project2_oidc_secret
|
||||
secret: project2-oidc-secret
|
||||
|
||||
- project:
|
||||
check:
|
||||
|
@ -8013,7 +8013,10 @@ class TestSecrets(ZuulTestCase):
|
||||
self.assertEqual(len(secrets), 1)
|
||||
for secret_name, secret_content in secrets.items():
|
||||
self._validate_oidc_token(
|
||||
secret_name, secret_content.value, self.history[0].uuid)
|
||||
# The var name is with '_' while the secret.name is with '-'
|
||||
# which should be used in the value of the 'sub' claim
|
||||
secret_name.replace('_', '-'),
|
||||
secret_content.value, self.history[0].uuid)
|
||||
|
||||
def test_oidc_multi(self):
|
||||
# Test that oidc token is generated correctly when there are
|
||||
@ -8060,20 +8063,23 @@ class TestSecrets(ZuulTestCase):
|
||||
'project2-oidc-secret', 'pre_playbooks'
|
||||
)[0]
|
||||
for secret_name, secret_content in parent_pre_secrets.items():
|
||||
|
||||
self._validate_oidc_token(
|
||||
secret_name, secret_content.value,
|
||||
# var: "login_secretx" -> secret: "project2-oidc-secretx"
|
||||
'project2-oidc-secret' + secret_name[-1],
|
||||
secret_content.value,
|
||||
self.history[1].uuid, "parent.yaml")
|
||||
parent_post_secrets = self._getSecrets(
|
||||
'project2-oidc-secret', 'post_playbooks'
|
||||
)[0]
|
||||
for secret_name, secret_content in parent_post_secrets.items():
|
||||
self._validate_oidc_token(
|
||||
secret_name, secret_content.value,
|
||||
'project2-oidc-secret' + secret_name[-1], secret_content.value,
|
||||
self.history[1].uuid, "parent.yaml")
|
||||
|
||||
for secret_name, secret_content in secrets.items():
|
||||
self._validate_oidc_token(
|
||||
secret_name, secret_content.value,
|
||||
'project2-oidc-secret' + secret_name[-1], secret_content.value,
|
||||
self.history[1].uuid)
|
||||
|
||||
def test_oidc_mix(self):
|
||||
@ -8107,7 +8113,8 @@ class TestSecrets(ZuulTestCase):
|
||||
del secrets['project2_secret']
|
||||
for secret_name, secret_content in secrets.items():
|
||||
self._validate_oidc_token(
|
||||
secret_name, secret_content.value, self.history[0].uuid)
|
||||
secret_name.replace('_', '-'),
|
||||
secret_content.value, self.history[0].uuid)
|
||||
|
||||
def test_oidc_iss_override(self):
|
||||
# Test that the custom 'iss' is allowed when configured
|
||||
@ -8137,7 +8144,8 @@ class TestSecrets(ZuulTestCase):
|
||||
}
|
||||
for secret_name, secret_content in secrets.items():
|
||||
self._validate_oidc_token(
|
||||
secret_name, secret_content.value, self.history[0].uuid,
|
||||
secret_name.replace('_', '-'),
|
||||
secret_content.value, self.history[0].uuid,
|
||||
expected_iss=expected_isses[secret_name])
|
||||
|
||||
def _validate_oidc_token(self, oidc_name, oidc_token, build_uuid,
|
||||
|
@ -3649,11 +3649,16 @@ class AnsibleJob(object):
|
||||
algorithm=algorithm)
|
||||
iat = int(time.time())
|
||||
ttl = oidc_config['ttl']
|
||||
if 'name' in oidc_config:
|
||||
# MODEL_API >= 35
|
||||
secret_name = oidc_config['name']
|
||||
else:
|
||||
secret_name = oidc_name
|
||||
exp = iat + ttl
|
||||
tenant = self.arguments['zuul']['tenant']
|
||||
canonical_project_name = \
|
||||
self.arguments['zuul']['project']['canonical_name']
|
||||
sub = f'secret:{tenant}/{canonical_project_name}/{oidc_name}'
|
||||
sub = f'secret:{tenant}/{canonical_project_name}/{secret_name}'
|
||||
iss = oidc_config.get('iss', self.arguments["zuul_root_url"])
|
||||
|
||||
payload = {
|
||||
|
@ -2826,6 +2826,9 @@ class Secret(ConfigObject):
|
||||
# max.
|
||||
secret_oidc['ttl'] = secret_oidc.get(
|
||||
'ttl', layout.tenant.default_oidc_ttl)
|
||||
# The original name needs to be passed for generating
|
||||
# the value of the 'sub' claim
|
||||
secret_oidc['name'] = self.name
|
||||
else:
|
||||
secret_oidc = {}
|
||||
|
||||
|
Reference in New Issue
Block a user