Skip to content
This repository was archived by the owner on Aug 7, 2023. It is now read-only.

Commit 06f3d00

Browse files
vuongxuongminhvuongxuongminh
authored
Merge pull request #2 from php-istio/bug/array-prototype-config-always-add-empty-array
Fix bug array prototype config always add empty array.
2 parents aac07ae + c7fbed5 commit 06f3d00

File tree

4 files changed

+172
-85
lines changed

4 files changed

+172
-85
lines changed

‎CHANGELOG.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
# Changelog
22

3+
## 1.0.2
4+
5+
* Fix: add child node `rules` array config to avoid empty array when using arrayPrototype.
6+
37
## 1.0.1
48

59
* Fix: auto add authenticator when not config.

‎src/DependencyInjection/Security/AuthenticatorFactory.php‎

Lines changed: 50 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ public function createAuthenticator(
3131
) {
3232
$authenticator = sprintf('security.authenticator.istio_jwt_authenticator.%s', $firewallName);
3333
$definition = new ChildDefinition('istio.jwt_authentication.authenticator');
34-
$definition->replaceArgument(0, $this->createUserIdentifierClaimMappings($container, $authenticator, $config));
34+
$definition->replaceArgument(0, $this->createUserIdentifierClaimMappings($container, $authenticator, $config['rules']));
3535
$definition->replaceArgument(1, new Reference($userProviderId));
3636
$container->setDefinition($authenticator, $definition);
3737

@@ -61,33 +61,40 @@ public function getKey()
6161
public function addConfiguration(NodeDefinition $builder)
6262
{
6363
$builder
64-
->fixXmlConfig('origin_token_header')
65-
->fixXmlConfig('origin_token_query_param')
66-
->fixXmlConfig('base64_header')
67-
->arrayPrototype()
68-
->addDefaultsIfNotSet()
69-
->children()
70-
->scalarNode('issuer')
71-
->cannotBeEmpty()
72-
->isRequired()
73-
->end()
74-
->scalarNode('user_identifier_claim')
75-
->cannotBeEmpty()
76-
->defaultValue('sub')
77-
->end()
78-
->arrayNode('origin_token_headers')
79-
->scalarPrototype()
80-
->cannotBeEmpty()
81-
->end()
82-
->end()
83-
->arrayNode('origin_token_query_params')
84-
->scalarPrototype()
85-
->cannotBeEmpty()
86-
->end()
87-
->end()
88-
->arrayNode('base64_headers')
89-
->scalarPrototype()
90-
->cannotBeEmpty()
64+
->fixXmlConfig('rule')
65+
->children()
66+
->arrayNode('rules')
67+
->isRequired()
68+
->cannotBeEmpty()
69+
->arrayPrototype()
70+
->fixXmlConfig('origin_token_header')
71+
->fixXmlConfig('origin_token_query_param')
72+
->fixXmlConfig('base64_header')
73+
->addDefaultsIfNotSet()
74+
->children()
75+
->scalarNode('issuer')
76+
->isRequired()
77+
->cannotBeEmpty()
78+
->end()
79+
->scalarNode('user_identifier_claim')
80+
->cannotBeEmpty()
81+
->defaultValue('sub')
82+
->end()
83+
->arrayNode('origin_token_headers')
84+
->scalarPrototype()
85+
->cannotBeEmpty()
86+
->end()
87+
->end()
88+
->arrayNode('origin_token_query_params')
89+
->scalarPrototype()
90+
->cannotBeEmpty()
91+
->end()
92+
->end()
93+
->arrayNode('base64_headers')
94+
->scalarPrototype()
95+
->cannotBeEmpty()
96+
->end()
97+
->end()
9198
->end()
9299
->end()
93100
->end()
@@ -97,53 +104,53 @@ public function addConfiguration(NodeDefinition $builder)
97104

98105
private function createUserIdentifierClaimMappings(
99106
ContainerBuilder $container,
100-
string $authenticatorName,
101-
array $config,
107+
string $authenticatorId,
108+
array $rules,
102109
): IteratorArgument {
103-
$extractorIdPrefix = sprintf('%s.payload_extractor', $authenticatorName);
110+
$extractorIdPrefix = sprintf('%s.payload_extractor', $authenticatorId);
104111
$mappings = [];
105112

106-
foreach ($config as $key => $item) {
113+
foreach ($rules as $key => $rule) {
107114
$extractor = null;
108115

109-
if (!empty($item['origin_token_headers'])) {
116+
if (!empty($rule['origin_token_headers'])) {
110117
$extractor = $this->createPayloadExtractor(
111118
$container,
112119
sprintf('%s.origin_token_headers.%s', $extractorIdPrefix, $key),
113120
'istio.jwt_authentication.payload_extractor.origin_token.header',
114-
$item['issuer'],
115-
$item['origin_token_headers']
121+
$rule['issuer'],
122+
$rule['origin_token_headers']
116123
);
117124
}
118125

119-
if (!empty($item['origin_token_query_params'])) {
126+
if (!empty($rule['origin_token_query_params'])) {
120127
$extractor = $this->createPayloadExtractor(
121128
$container,
122129
sprintf('%s.origin_token_query_params.%s', $extractorIdPrefix, $key),
123130
'istio.jwt_authentication.payload_extractor.origin_token.query_param',
124-
$item['issuer'],
125-
$item['origin_token_query_params']
131+
$rule['issuer'],
132+
$rule['origin_token_query_params']
126133
);
127134
}
128135

129-
if (!empty($item['base64_headers'])) {
136+
if (!empty($rule['base64_headers'])) {
130137
$extractor = $this->createPayloadExtractor(
131138
$container,
132139
sprintf('%s.base64_headers.%s', $extractorIdPrefix, $key),
133140
'istio.jwt_authentication.payload_extractor.base64_header',
134-
$item['issuer'],
135-
$item['base64_headers']
141+
$rule['issuer'],
142+
$rule['base64_headers']
136143
);
137144
}
138145

139146
if (null === $extractor) {
140147
throw new InvalidConfigurationException(sprintf('`%s`: at least once `origin_token_headers`, `origin_token_query_params`, `base64_headers` should be config when using', $this->getKey()));
141148
}
142149

143-
$mappingId = sprintf('%s.user_identifier_claim_mapping.%s', $authenticatorName, $key);
150+
$mappingId = sprintf('%s.user_identifier_claim_mapping.%s', $authenticatorId, $key);
144151
$mappings[] = new Reference($mappingId);
145152
$mappingDefinition = new Definition(UserIdentifierClaimMapping::class);
146-
$mappingDefinition->setArgument(0, $item['user_identifier_claim']);
153+
$mappingDefinition->setArgument(0, $rule['user_identifier_claim']);
147154
$mappingDefinition->setArgument(1, $extractor);
148155
$container->setDefinition($mappingId, $mappingDefinition);
149156
}

‎tests/TestKernel.php‎

Lines changed: 25 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -68,29 +68,41 @@ function (ContainerBuilder $container) {
6868
'provider' => 'istio',
6969
'stateless' => true,
7070
'istio_jwt_authenticator' => [
71-
[
72-
'issuer' => 'issuer_1',
73-
'user_identifier_claim' => 'id_1',
74-
'origin_token_query_params' => ['token'],
75-
],
76-
[
77-
'issuer' => 'issuer_2',
78-
'user_identifier_claim' => 'id_2',
79-
'base64_headers' => ['x-istio-jwt-payload'],
71+
'rules' => [
72+
[
73+
'issuer' => 'issuer_1',
74+
'user_identifier_claim' => 'id_1',
75+
'origin_token_query_params' => ['token'],
76+
],
77+
[
78+
'issuer' => 'issuer_2',
79+
'user_identifier_claim' => 'id_2',
80+
'base64_headers' => ['x-istio-jwt-payload'],
81+
],
8082
],
8183
],
8284
],
8385
'test2' => [
8486
'provider' => 'memory',
8587
'stateless' => true,
8688
'istio_jwt_authenticator' => [
87-
[
88-
'issuer' => 'issuer_2',
89-
'user_identifier_claim' => 'id_2',
90-
'origin_token_headers' => ['authorization'],
89+
'rules' => [
90+
[
91+
'issuer' => 'issuer_2',
92+
'user_identifier_claim' => 'id_2',
93+
'origin_token_headers' => ['authorization'],
94+
],
9195
],
9296
],
9397
],
98+
// Test not affect another authenticator
99+
'test3' => [
100+
'provider' => 'istio',
101+
'stateless' => true,
102+
'http_basic' => [
103+
'realm' => 'Test',
104+
],
105+
],
94106
],
95107
'providers' => [
96108
'istio' => [

‎tests/Unit/DepdendencyInjection/Security/AuthenticatorFactoryTest.php‎

Lines changed: 93 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -67,8 +67,10 @@ public function testExceptionWhenCallCreate()
6767
public function testCreateAuthenticator()
6868
{
6969
$config = [
70-
['issuer' => 'test', 'origin_token_headers' => ['authorization'], 'user_identifier_claim' => 'sub'],
71-
['issuer' => 'test2', 'origin_token_query_params' => ['token'], 'user_identifier_claim' => 'sub'],
70+
'rules' => [
71+
['issuer' => 'test', 'origin_token_headers' => ['authorization'], 'user_identifier_claim' => 'sub'],
72+
['issuer' => 'test2', 'origin_token_query_params' => ['token'], 'user_identifier_claim' => 'sub'],
73+
],
7274
];
7375

7476
$this->executeCreate($config);
@@ -84,7 +86,7 @@ public function testCreateAuthenticator()
8486
public function testThrowExceptionWhenCreateAuthenticatorWithNoneExtractor()
8587
{
8688
$this->expectException(InvalidConfigurationException::class);
87-
$this->executeCreate([['issuer' => 'test']]);
89+
$this->executeCreate(['rules' => ['issuer' => 'test']]);
8890
}
8991

9092
private function executeCreate(array $config)
@@ -98,37 +100,69 @@ public function validConfigurations(): array
98100
return [
99101
[
100102
[
101-
[
102-
'issuer' => 'example',
103+
'rules' => [
104+
[
105+
'issuer' => 'example',
106+
],
103107
],
104108
],
105109
[
106-
[
107-
'issuer' => 'example',
108-
'user_identifier_claim' => 'sub',
109-
'origin_token_headers' => [],
110-
'origin_token_query_params' => [],
111-
'base64_headers' => [],
110+
'rules' => [
111+
[
112+
'issuer' => 'example',
113+
'user_identifier_claim' => 'sub',
114+
'origin_token_headers' => [],
115+
'origin_token_query_params' => [],
116+
'base64_headers' => [],
117+
],
112118
],
113119
],
114120
],
115121
[
116122
[
117-
[
118-
'issuer' => 'example',
119-
'user_identifier_claim' => 'id',
120-
'origin_token_headers' => ['authorization'],
121-
'origin_token_query_params' => ['token'],
122-
'base64_headers' => ['x-istio-jwt-payload'],
123+
'rules' => [
124+
[
125+
'issuer' => 'example',
126+
'user_identifier_claim' => 'id',
127+
'origin_token_headers' => ['authorization'],
128+
'origin_token_query_params' => ['token'],
129+
'base64_headers' => ['x-istio-jwt-payload'],
130+
],
123131
],
124132
],
125133
[
126-
[
127-
'issuer' => 'example',
128-
'user_identifier_claim' => 'id',
129-
'origin_token_headers' => ['authorization'],
130-
'origin_token_query_params' => ['token'],
131-
'base64_headers' => ['x-istio-jwt-payload'],
134+
'rules' => [
135+
[
136+
'issuer' => 'example',
137+
'user_identifier_claim' => 'id',
138+
'origin_token_headers' => ['authorization'],
139+
'origin_token_query_params' => ['token'],
140+
'base64_headers' => ['x-istio-jwt-payload'],
141+
],
142+
],
143+
],
144+
],
145+
[
146+
[
147+
'rules' => [
148+
[
149+
'issuer' => 'example',
150+
'user_identifier_claim' => 'id',
151+
'origin_token_header' => ['authorization'],
152+
'origin_token_query_param' => ['token'],
153+
'base64_header' => ['x-istio-jwt-payload'],
154+
],
155+
],
156+
],
157+
[
158+
'rules' => [
159+
[
160+
'issuer' => 'example',
161+
'user_identifier_claim' => 'id',
162+
'origin_token_headers' => ['authorization'],
163+
'origin_token_query_params' => ['token'],
164+
'base64_headers' => ['x-istio-jwt-payload'],
165+
],
132166
],
133167
],
134168
],
@@ -138,34 +172,64 @@ public function validConfigurations(): array
138172
public function invalidConfigurations(): array
139173
{
140174
return [
175+
[
176+
[],
177+
],
141178
[
142179
[
143-
['issuer' => ''],
180+
'rules' => [],
144181
],
145182
],
146183
[
147184
[
148-
['issuer' => 'example', 'user_identifier_claim' => ''],
185+
'rules' => [[]],
149186
],
150187
],
151188
[
152189
[
153-
['issuer' => '', 'user_identifier_claim' => 'id'],
190+
'rules' => ['issuer' => ''],
154191
],
155192
],
156193
[
157194
[
158-
['issuer' => 'example', 'origin_token_headers' => ['']],
195+
'rules' => [
196+
['issuer' => ''],
197+
],
159198
],
160199
],
161200
[
162201
[
163-
['issuer' => 'example', 'origin_token_query_params' => ['']],
202+
'rules' => [
203+
['issuer' => 'example', 'user_identifier_claim' => ''],
204+
],
164205
],
165206
],
166207
[
167208
[
168-
['issuer' => 'example', 'base64_headers' => ['']],
209+
'rules' => [
210+
['issuer' => '', 'user_identifier_claim' => 'id'],
211+
],
212+
],
213+
],
214+
[
215+
[
216+
'rules' => [
217+
['issuer' => 'example', 'origin_token_headers' => ['']],
218+
],
219+
],
220+
],
221+
[
222+
[
223+
'rules' => [
224+
['issuer' => 'example', 'origin_token_query_params' => ['']],
225+
],
226+
],
227+
],
228+
[
229+
[
230+
'rules' => [
231+
['issuer' => 'example', 'base64_headers' => ['']],
232+
],
169233
],
170234
],
171235
];

0 commit comments

Comments
 (0)