-
Notifications
You must be signed in to change notification settings - Fork 47
Make new dashboard client credentials configurable on bootstrap #1185
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
SteDev2
wants to merge
27
commits into
develop
Choose a base branch
from
init-dashboard
base: develop
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+660
−3
Open
Changes from all commits
Commits
Show all changes
27 commits
Select commit
Hold shift + click to select a range
35a2074
Add dashboard properties
SteDev2 5c4a02c
Refactor DashboardProperties
SteDev2 e355c12
Add dashboard properties validator
SteDev2 0f7a556
Adds dashboard enabled flag
SteDev2 bd9b92b
Fix typo
SteDev2 6f9f814
Check dashboard configuration record on DB
SteDev2 509bfe0
Limit max client secret length
SteDev2 e0f5a61
Removes dashboard client base URL from config
SteDev2 221e380
Add unit tests for DashboardConfigService
SteDev2 2368b2e
Add unit tests for DashboardConfigValidator
SteDev2 9a46b13
Update CLIENT_ID_REGEX to match UUID
SteDev2 707936d
Update DashboardConfigValidator
SteDev2 dfd8ec5
Fix scope for new DashboardClient
SteDev2 ce43a48
Add tests in DashboardConfigServiceTest
SteDev2 c8e445b
Merge branch 'develop' into init-dashboard
SteDev2 212ae55
Remove unused imports
SteDev2 68af9af
Add StartupRunnerTests to verify dashboard initialization
SteDev2 a76e6ec
Refactor StartupRunner
SteDev2 c78c608
Add @Transactional annotation
SteDev2 287f255
Refactor exception
SteDev2 2e58c58
Add StartupRunnerWithoutConfigurationTests
SteDev2 9824e29
Add tests for StartupRunner and refactor
SteDev2 aa9e232
Add mock behavior for clientService in StartupRunnerTests
SteDev2 2490e6b
Refactor and improve tests
SteDev2 c158b3b
Refactor dashboard configuration and update tests
SteDev2 48e0c96
Remove useless import
SteDev2 d4ad58e
Refactor variable naming
SteDev2 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
46 changes: 46 additions & 0 deletions
46
...c/main/java/it/infn/mw/iam/api/client/management/validation/DashboardConfigValidator.java
This file contains hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,46 @@ | ||
|
|
||
| /** | ||
| * Copyright (c) Istituto Nazionale di Fisica Nucleare (INFN). 2016-2021 | ||
| * | ||
| * Licensed under the Apache License, Version 2.0 (the "License"); | ||
| * you may not use this file except in compliance with the License. | ||
| * You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
| package it.infn.mw.iam.api.client.management.validation; | ||
|
|
||
| import javax.validation.ConstraintValidator; | ||
| import javax.validation.ConstraintValidatorContext; | ||
|
|
||
| import org.springframework.context.annotation.Scope; | ||
| import org.springframework.stereotype.Component; | ||
|
|
||
| import it.infn.mw.iam.config.IamProperties.DashboardProperties; | ||
|
|
||
| @Component | ||
| @Scope("prototype") | ||
| public class DashboardConfigValidator implements ConstraintValidator<ValidDashboard, DashboardProperties> { | ||
|
|
||
| private static final String CLIENT_ID_REGEX = "^[a-zA-Z0-9\\-._~]{4,256}$"; | ||
| private static final String CLIENT_SECRET_REGEX = "^[a-zA-Z0-9\\-._~]{32,72}$"; | ||
|
|
||
| @Override | ||
| public boolean isValid(DashboardProperties dashboardProperties, ConstraintValidatorContext context) { | ||
| if (dashboardProperties == null || !dashboardProperties.isEnabled()) { | ||
| return true; | ||
| } | ||
| boolean validClientId = dashboardProperties.getClientId() != null | ||
| && dashboardProperties.getClientId().matches(CLIENT_ID_REGEX); | ||
| boolean validClientSecret = dashboardProperties.getClientSecret() != null | ||
| && dashboardProperties.getClientSecret().matches(CLIENT_SECRET_REGEX); | ||
|
|
||
| return validClientId && validClientSecret; | ||
| } | ||
| } |
36 changes: 36 additions & 0 deletions
36
...service/src/main/java/it/infn/mw/iam/api/client/management/validation/ValidDashboard.java
This file contains hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| /** | ||
| * Copyright (c) Istituto Nazionale di Fisica Nucleare (INFN). 2016-2021 | ||
| * | ||
| * Licensed under the Apache License, Version 2.0 (the "License"); | ||
| * you may not use this file except in compliance with the License. | ||
| * You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
| package it.infn.mw.iam.api.client.management.validation; | ||
|
|
||
| import static java.lang.annotation.ElementType.FIELD; | ||
| import static java.lang.annotation.RetentionPolicy.RUNTIME; | ||
|
|
||
| import java.lang.annotation.Retention; | ||
| import java.lang.annotation.Target; | ||
|
|
||
| import javax.validation.Constraint; | ||
| import javax.validation.Payload; | ||
|
|
||
| @Retention(RUNTIME) | ||
| @Target({FIELD}) | ||
| @Constraint(validatedBy = DashboardConfigValidator.class) | ||
| public @interface ValidDashboard { | ||
| String message() default "Invalid dashboard client configuration"; | ||
|
|
||
| Class <?>[] groups() default {}; | ||
|
|
||
| Class<? extends Payload>[] payload() default {}; | ||
| } |
This file contains hidden or 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 file contains hidden or 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
52 changes: 52 additions & 0 deletions
52
iam-login-service/src/main/java/it/infn/mw/iam/core/util/StartupRunner.java
This file contains hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,52 @@ | ||
| /** | ||
| * Copyright (c) Istituto Nazionale di Fisica Nucleare (INFN). 2016-2021 | ||
| * | ||
| * Licensed under the Apache License, Version 2.0 (the "License"); | ||
| * you may not use this file except in compliance with the License. | ||
| * You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
|
|
||
| package it.infn.mw.iam.core.util; | ||
|
|
||
| import org.slf4j.Logger; | ||
| import org.slf4j.LoggerFactory; | ||
| import org.springframework.boot.ApplicationArguments; | ||
| import org.springframework.boot.ApplicationRunner; | ||
| import org.springframework.stereotype.Component; | ||
|
|
||
| import it.infn.mw.iam.dashboard.DashboardConfigService; | ||
|
|
||
| @Component | ||
| public class StartupRunner implements ApplicationRunner { | ||
|
|
||
| private static final Logger LOG = LoggerFactory.getLogger(StartupRunner.class); | ||
|
|
||
| private final DashboardConfigService dashboardConfigService; | ||
|
|
||
| public StartupRunner(DashboardConfigService dashboardConfigService) { | ||
| this.dashboardConfigService = dashboardConfigService; | ||
| } | ||
|
|
||
| @Override | ||
| public void run(ApplicationArguments args) { | ||
| if (!dashboardConfigService.isEnabled()) { | ||
| LOG.info( | ||
| "Dashboard client is disabled, skipping checks for the dashboard client properties and the presence of the record for the dashboard client"); | ||
| return; | ||
| } | ||
|
|
||
| boolean isValid = dashboardConfigService.init(); | ||
| if (!isValid) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. checking the code of the service, I see |
||
| throw new IllegalStateException( | ||
| "Dashboard client record does not exist or is not valid. Please check the dashboard client properties and ensure that a record with the specified client id, client secret and redirect uri exists in the database"); | ||
| } | ||
| } | ||
| } | ||
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd simplify this message into "Skipping dashboard client initialization." The perfect message should include something like "Read more info about here: " + URL of an updated IAM documentation.