Skip to content
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

feat(jans-keycloak-integration): enhancements to keycloak integration #8614 #8747

Merged
merged 40 commits into from
Jun 24, 2024

Conversation

uprightech
Copy link
Contributor

@uprightech uprightech commented Jun 20, 2024

Various bugfixes , enhancements and refactorings to do for keycloak integration

  • Implement and integrate a protocol mapper
  • Implement a core jans spi containing common functionality that can be accessed by other spis
  • Merge all spis into one to ease deployment and dependency management
  • begin writing an automated test strategy for KC SAML

* updated the keycloak configuration file to reflect the  configuration for the storage-spi

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

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

* added persistence manager configuration for protocol mapper

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

Signed-off-by: Rolain Djeumen <uprightech@gmail.com>
* added dependencies to protocol mapper
* added protocol mapper main class

Signed-off-by: Rolain Djeumen <uprightech@gmail.com>
* added relevant models to fetch user attributes
* refactored the db configuration classes

Signed-off-by: Rolain Djeumen <uprightech@gmail.com>
* created maven project for janssen spi bundle

Signed-off-by: Rolain Djeumen <uprightech@gmail.com>
* added dependencies xml

Signed-off-by: Rolain Djeumen <uprightech@gmail.com>
* added support for new protocol mapper in job scheduler
* fixed typo in application shutdown log message

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


* added support for the protocol-mapper in job-scheduler configuration
* fixed issue in  job-scheduler logging configuration that caused too many log files to be created

Signed-off-by: Rolain Djeumen <uprightech@gmail.com>
* additions to the spi bundle pom file

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


* added protocol mapper implementation

Signed-off-by: Rolain Djeumen <uprightech@gmail.com>
…ation #8614

* added thin bridge spi provider
* added models for thin bridge provider

Signed-off-by: Rolain Djeumen <uprightech@gmail.com>
…ation #8614

* moved authenticator spi to spi module
* minor refactoring to the authenticator spi

Signed-off-by: Rolain Djeumen <uprightech@gmail.com>
…ation #8614

* moved authenticator rest service spi to spi module

Signed-off-by: Rolain Djeumen <uprightech@gmail.com>
…ation #8614

* added new storage provider implementation

Signed-off-by: Rolain Djeumen <uprightech@gmail.com>
…ation #8614

* added missing files to spi

Signed-off-by: Rolain Djeumen <uprightech@gmail.com>
…ation #8614

* added resource files to spi module

Signed-off-by: Rolain Djeumen <uprightech@gmail.com>
…ation #8614

* bump spi version to 1.1.3-SNAPSHOT
* removed protocol-mapper PoC from build modules

Signed-off-by: Rolain Djeumen <uprightech@gmail.com>
…ation #8614

* minor bugfix to scheduler. did not show fatal startup errors in log file

Signed-off-by: Rolain Djeumen <uprightech@gmail.com>
…ation #8614

*fix for fatal errors which don't still appear in the logs

Signed-off-by: Rolain Djeumen <uprightech@gmail.com>
…ation #8614

* further housekeeping in job-scheduler

Signed-off-by: Rolain Djeumen <uprightech@gmail.com>
…ation #8614

* fixed bug in user storage spi preventing authentication in new version of keycloak

Signed-off-by: Rolain Djeumen <uprightech@gmail.com>
…ation #8614

* have scheduler create saml clients with document and assertion signing as default configuration

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

dryrunsecurity bot commented Jun 20, 2024

Hi there 👋, @DryRunSecurity here, below is a summary of our analysis and findings.

DryRun Security Status Findings
Server-Side Request Forgery Analyzer 0 findings
Configured Codepaths Analyzer 0 findings
Secrets Analyzer 0 findings
Authn/Authz Analyzer 9 findings
SQL Injection Analyzer 0 findings
Sensitive Files Analyzer 3 findings
IDOR Analyzer 0 findings

Note

🟢 Risk threshold not exceeded.

Change Summary (click to expand)

The following is a summary of changes in this pull request made by me, your security buddy 🤖. Note that this summary is auto-generated and not meant to be a definitive list of security issues but rather a helpful summary from a security perspective.

Summary:

The provided code changes cover various aspects of the Jans Keycloak integration, including updates to the SAML client configuration, error handling and logging improvements, attribute mapping management, and package refactoring. From an application security perspective, the changes generally appear to be positive and focused on improving the security and reliability of the integration.

Key security-related improvements include:

  1. Enabling SAML document and assertion signing by default, which enhances the security of the SAML integration.
  2. Improving error handling and logging to prevent sensitive information leakage and ensure more consistent and informative error reporting.
  3. Careful management of SAML attribute mappings, including handling of released attributes and unique protocol mapper naming.
  4. Secure handling of OIDC token requests, user information, and metadata caching.

While the changes do not introduce any obvious security vulnerabilities, it's important to consider the broader context and ensure that the application's security practices are consistently applied across all components of the Keycloak integration. This includes thorough input validation, secure data handling, and ongoing monitoring and testing to identify and address any potential security risks.

Files Changed:

  1. jans-keycloak-integration/job-scheduler/src/main/java/io/jans/kc/api/admin/client/model/ManagedSamlClient.java: Enabled SAML document and assertion signing by default, improving the overall security of the SAML integration.
  2. jans-keycloak-integration/job-scheduler/src/main/java/io/jans/kc/scheduler/App.java: Improved error handling and logging to ensure more consistent and informative error reporting.
  3. jans-keycloak-integration/job-scheduler/src/main/java/io/jans/kc/api/admin/client/model/ProtocolMapper.java: Added a method to set the "jans.attribute.name" configuration parameter for protocol mappers, which is important for managing attribute mappings.
  4. jans-keycloak-integration/job-scheduler/src/main/java/io/jans/kc/scheduler/job/impl/QuartzJobWrapper.java: Improved exception handling and logging for scheduled job execution.
  5. jans-keycloak-integration/job-scheduler/src/main/resources/logback.xml.sample: Updated the log file naming pattern to use the full month name instead of the abbreviated version.
  6. jans-keycloak-integration/job-scheduler/src/main/resources/config.properties.sample: Updated the SAML user attribute mapper configuration, which could have security implications and should be reviewed.
  7. jans-keycloak-integration/job-scheduler/src/main/java/io/jans/kc/scheduler/TrustRelationshipSyncJob.java: Improved the management of SAML clients, including the deletion of unused clients and the updating of existing clients to reflect changes in the associated Janssen trust relationships.
  8. jans-keycloak-integration/spi/src/assembly/dependencies.xml: Added a new file set to include additional files in the final assembly package.
  9. jans-keycloak-integration/spi/src/main/java/io/jans/kc/model/JansUserAttributeModel.java: Improved the conversion of Jans user attributes to SAML attributes.
  10. jans-keycloak-integration/pom.xml: Updated the project dependencies and module structure, which should be reviewed for any security implications.
  11. jans-keycloak-integration/spi/src/main/java/io/jans/kc/model/JansUserModel.java: Implemented a read-only user model, which can have security benefits by limiting the potential attack surface.
  12. jans-keycloak-integration/spi/src/main/java/io/jans/kc/oidc/ directory: Various changes to OIDC-related classes, including package refactoring and the addition of new interfaces and error handling classes. These changes should be reviewed to ensure the continued security of the OIDC integration.

Powered by DryRun Security

…tion #8614

* removed reference to protocol-mapper poc submodule

Signed-off-by: Rolain Djeumen <uprightech@gmail.com>
moabu
moabu previously approved these changes Jun 20, 2024
…ation #8614

* removed reference to storage-spi module
* restored job-scheduler module in build pom

Signed-off-by: Rolain Djeumen <uprightech@gmail.com>
…ation #8614

* removed authenticator source as it was moved to spi

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

sonarcloud bot commented Jun 24, 2024

…ation #8614

* fixes suggested by static analyser

Signed-off-by: Rolain Djeumen <uprightech@gmail.com>
…ation #8614

Signed-off-by: Rolain Djeumen <uprightech@gmail.com>
…ation #8614

Signed-off-by: Rolain Djeumen <uprightech@gmail.com>
…ation #8614

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

sonarcloud bot commented Jun 24, 2024

Quality Gate Passed Quality Gate passed for 'keycloak-integration-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

@moabu moabu merged commit cfdf223 into main Jun 24, 2024
10 checks passed
@moabu moabu deleted the issue_8614 branch June 24, 2024 17:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feat(jans-keycloak-integration): perform various fixes and enhancements to keycloak integration
2 participants