8000 fix(jans-config-api): security bugfixes #8963 by uprightech · Pull Request #8974 · JanssenProject/jans · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

fix(jans-config-api): security bugfixes #8963 #8974

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 14 commits into from
Jul 18, 2024
Merged

fix(jans-config-api): security bugfixes #8963 #8974

merged 14 commits into from
Jul 18, 2024

Conversation

uprightech
Copy link
Contributor

Prepare

Description

Target issue

closes #issue-number-here

Implementation Details

Test and Document the changes

  • [x ] Static code analysis has been run locally and issues have been fixed
  • Relevant unit and integration tests have been added/updated
  • Relevant documentation has been updated if any (i.e. user guides, installation and configuration guides, technical design docs etc)

Please check the below before submitting your PR. The PR will not be merged if there are no commits that start with docs: to indicate documentation changes or if the below checklist is not selected.

  • I confirm that there is no impact on the docs due to the code changes in this PR.

uprightech added 13 commits April 3, 2024 12:04
@uprightech
fix(jans-linux-setup): improper scim configuration for jans kc #8210
ffe3dbe 
* updated the keycloak configuration file to reflect the  configuration for the storage-spi

Signed-off-by: Rolain Djeumen <uprightech@gmail.com>
@uprightech
Merge remote-tracking branch 'origin/main'
76e0414
@uprightech
Merge remote-tracking branch 'origin/main'
97f3c02
@uprightech
chore(jans-keycloak-integration): bump kc version to 24.0.0 #8315
fadf1f2 
Signed-off-by: Rolain Djeumen <uprightech@gmail.com>
@uprightech
Merge remote-tracking branch 'origin/main'
e5d4afb 
Signed-off-by: Rolain Djeumen <uprightech@gmail.com>
@uprightech
Merge remote-tracking branch 'origin/main'
9790b10
@uprightech
Merge remote-tracking branch 'origin/main'
8b77454
@uprightech
Merge remote-tracking branch 'origin/main'
2e0cd48
@uprightech
Merge remote-tracking branch 'origin/main'
f6cf31b
@uprightech
Merge remote-tracking branch 'origin/main'
3c80ffb
@uprightech
Merge remote-tracking branch 'origin/main'
be4b230
@uprightech
fix(jans-config-api): broken build of saml config-api plugin #8963
b3774db 
* begin removing references to the metadata timer (functionality moved to the scheduler)

Signed-off-by: Rolain Djeumen <uprightech@gmail.com>
@uprightech
fix(jans-config-api): broken build of saml config-api plugin #8963
97435cf 
* removed references to MetadataValidationTimer
* refactored saml inbount and saml idp rest resource providers

Signed-off-by: Rolain Djeumen <uprightech@gmail.com>
@dryrunsecurity DryRunSecurity
Copy link
dryrunsecurity bot commented Jul 17, 2024 •
edited
Loading

DryRun Security Summary

The pull request focuses on improving the management of SAML Identity Providers and Service Providers in the Jans Config API application, including the removal of metadata file processing functionality, simplification of metadata file handling, and improvements to logging, potentially reducing the attack surface and mitigating security risks.

Expand for full summary

Summary:

The code changes in this pull request are primarily focused on the management of SAML (Security Assertion Markup Language) Identity Providers (IDPs) and Service Providers (SPs) in the Jans Config API application. The key changes include the removal of various methods responsible for processing unprocessed metadata files, simplification of metadata file handling, and improvements to logging.

From an application security perspective, the removal of the metadata file processing functionality could potentially reduce the attack surface and mitigate security risks associated with handling untrusted metadata files. However, it's important to ensure that the application still has alternative mechanisms in place to properly validate and update SAML metadata to maintain the security and integrity of the SAML integration.

The code changes also include updates to the SAML configuration endpoints, which are crucial for maintaining proper access control and security configurations. It's essential to thoroughly review these changes and test the application to ensure that the SAML-related functionality continues to meet the security requirements.

Overall, the changes appear to be focused on improving the SAML-related functionality and security of the Jans Config API. While the changes do not introduce any obvious security vulnerabilities, it's important to review the broader context of the application and ensure that the SAML integration and metadata handling processes are secure and robust.

Files Changed:

  1. jans-config-api/plugins/kc-saml-plugin/src/main/java/io/jans/configapi/plugin/saml/rest/IdpResource.java:

    • Removal of the processMetadataFiles() method, which was used to process unprocessed IDP metadata files.
    • Changes in the deleteIdentityProvider() method, including logging and null checks.

  2. jans-config-api/plugins/kc-saml-plugin/src/main/java/io/jans/configapi/plugin/saml/rest/TrustRelationshipResource.java:

    • Removal of the processMetadataFiles() method, which was responsible for processing unprocessed SP metadata files.
    • Validation of the SpMetaDataSourceType to ensure that the appropriate metadata is provided.

  3. jans-config-api/plugins/kc-saml-plugin/src/main/java/io/jans/configapi/plugin/saml/configuration/SamlAppInitializer.java:

    • Removal of the MetadataValidationTimer class, which was responsible for initializing a timer to validate SAML metadata.
    • Initialization and shutdown of the SAML plugin.

  4. jans-config-api/plugins/docs/kc-saml-plugin-swagger.yaml:

    • Removal of two endpoints related to processing IDP and SP metadata files.
    • Updates to the existing SAML configuration and trust relationship management endpoints.

  5. jans-config-api/plugins/kc-saml-plugin/src/main/java/io/jans/configapi/plugin/saml/service/IdpService.java:

    • Removal of the processUnprocessedIdpMetadataFiles() method.
    • Modification of the getSpMetadata() method to handle a null IdentityProvider object.

  6. jans-config-api/plugins/kc-saml-plugin/src/main/java/io/jans/configapi/plugin/saml/service/IdentityProviderService.java:

    • Removal of the MetadataValidationTimer class and its associated functionality.
    • Simplification of the saveIdpMetaDataFileSourceTypeFile method.

  7. jans-config-api/plugins/kc-saml-plugin/src/main/java/io/jans/configapi/plugin/saml/service/SamlService.java:

    • Removal of the MetadataValidationTimer class.
    • Simplified saveSpMetaDataFileSourceTypeFile method.
    • Removal of the processUnprocessedSpMetadataFiles method.
    • Improved logging statements.

Code Analysis

We ran 9 analyzers against 8 files and 1 analyzer had findings. 8 analyzers had no findings.

Analyzer Findings
Authn/Authz Analyzer 4 findings

Riskiness

🟢 Risk threshold not exceeded.

View PR in the DryRun Dashboard.

@mo-auto mo-auto added comp-jans-config-api Component affected by issue or PR kind-bug Issue or PR is a bug in existing functionality labels Jul 17, 2024
@sonarqubecloud SonarQubeCloud
Copy link

Quality Gate Passed Quality Gate passed for 'jans-config-api-parent'

Issues
6 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

@uprightech uprightech requested a review from moabu July 18, 2024 08:48
@moabu moabu merged commit 974040e into main Jul 18, 2024
10 of 11 checks passed
@moabu moabu deleted the issue_8963 branch July 18, 2024 08:57
@sonarqubecloud SonarQubeCloud
Copy link

Quality Gate Passed Quality Gate passed for 'jans-core'

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

@sonarqubecloud SonarQubeCloud
Copy link

Quality Gate Passed Quality Gate passed for 'jans-pycloudlib'

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

@sonarqubecloud SonarQubeCloud
Copy link

Quality Gate Passed Quality Gate passed for 'keycloak-integration-parent'

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

yuriyz pushed a commit that referenced this pull request Nov 7, 2024
@uprightech @moabu
fix(jans-config-api): security bugfixes #8963 (#8974)
0857961 
* fix(jans-linux-setup): improper scim configuration for jans kc #8210
* updated the keycloak configuration file to reflect the  configuration for the storage-spi

Signed-off-by: Rolain Djeumen <uprightech@gmail.com>

* chore(jans-keycloak-integration): bump kc version to 24.0.0 #8315

Signed-off-by: Rolain Djeumen <uprightech@gmail.com>

* fix(jans-config-api): broken build of saml config-api plugin #8963
* begin removing references to the metadata timer (functionality moved to the scheduler)

Signed-off-by: Rolain Djeumen <uprightech@gmail.com>

* fix(jans-config-api): broken build of saml config-api plugin #8963
* removed references to MetadataValidationTimer
* refactored saml inbount and saml idp rest resource providers

Signed-off-by: Rolain Djeumen <uprightech@gmail.com>

---------

Signed-off-by: Rolain Djeumen <uprightech@gmail.com>
Co-authored-by: Mohammad Abudayyeh <47318409+moabu@users.noreply.github.com>
Former-commit-id: 974040e
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pujavs pujavs pujavs approved these changes

@moabu moabu moabu approved these changes

@yuriyz yuriyz Awaiting requested review from yuriyz yuriyz is a code owner

@yurem yurem Awaiting requested review from yurem yurem is a code owner

Assignees
No one assigned
Labels
comp-jans-config-api Component affected by issue or PR kind-bug Issue or PR is a bug in existing functionality
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@uprightech @pujavs @moabu @mo-auto
0