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

fix(config-ap): lock audit endpoint parameter declaration error#9460 #9473

Merged
merged 75 commits into from
Sep 12, 2024

Conversation

pujavs
Copy link
Contributor

@pujavs pujavs commented Sep 12, 2024

Prepare


Description

Target issue

closes #9460

Implementation Details


Test and Document the changes

  • 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.

Signed-off-by: pujavs <pujas.works@gmail.com>
Signed-off-by: pujavs <pujas.works@gmail.com>
Signed-off-by: pujavs <pujas.works@gmail.com>
Signed-off-by: pujavs <pujas.works@gmail.com>
Signed-off-by: pujavs <pujas.works@gmail.com>
Signed-off-by: pujavs <pujas.works@gmail.com>
Signed-off-by: pujavs <pujas.works@gmail.com>
Signed-off-by: pujavs <pujas.works@gmail.com>
Signed-off-by: pujavs <pujas.works@gmail.com>
Signed-off-by: pujavs <pujas.works@gmail.com>
Signed-off-by: pujavs <pujas.works@gmail.com>
Signed-off-by: pujavs <pujas.works@gmail.com>
Signed-off-by: pujavs <pujas.works@gmail.com>
Signed-off-by: pujavs <pujas.works@gmail.com>
Signed-off-by: pujavs <pujas.works@gmail.com>
Signed-off-by: pujavs <pujas.works@gmail.com>
Signed-off-by: pujavs <pujas.works@gmail.com>
Signed-off-by: pujavs <pujas.works@gmail.com>
Signed-off-by: pujavs <pujas.works@gmail.com>
…ssets forconfig-api #9403

Signed-off-by: pujavs <pujas.works@gmail.com>
Signed-off-by: pujavs <pujas.works@gmail.com>
Signed-off-by: pujavs <pujas.works@gmail.com>
Signed-off-by: pujavs <pujas.works@gmail.com>
Signed-off-by: pujavs <pujas.works@gmail.com>
Signed-off-by: pujavs <pujas.works@gmail.com>
Copy link

dryrunsecurity bot commented Sep 12, 2024

DryRun Security Summary

The provided code changes involve updates to the Jans Config API's Lock plugin, including modifications to the input parameters for some of the API endpoints related to lock audit functionality, and updates to the visibility and editability of attributes in the application.

Expand for full summary

Summary:

The provided code changes involve updates to the Jans Config API's Lock plugin, including modifications to the input parameters for some of the API endpoints related to lock audit functionality. The changes move the eventStartDate and eventEndDate parameters from the path to the query parameters, which is a common practice to improve the flexibility and maintainability of the API. From an application security perspective, these changes do not introduce any obvious security concerns, as the API endpoints are already protected by OAuth2 security, and the input parameters are standard date-time values in ISO8601 format.

However, it's worth reviewing the overall API design and implementation to ensure that proper input validation, authorization, and error handling are in place to mitigate potential security risks. Additionally, the changes in the AuditResource class appear to be focused on improving the robustness and error handling of the date range filtering functionality, which is a positive step from an application security perspective.

Another change involves updating the visibility and editability of attributes in the application, which could impact the access control and authorization mechanisms. It's important to ensure that these changes align with the security requirements and policies of the application and that they don't introduce any unintended security vulnerabilities.

Files Changed:

  1. jans-config-api/plugins/docs/lock-plugin-swagger.yaml:

    • The changes update the input parameters for the /lock/audit/health/{eventStartDate}-{eventEndDate}, /lock/audit/log/{eventStartDate}-{eventEndDate}, and /lock/audit/telemetry/{eventStartDate}-{eventEndDate} endpoints, moving the eventStartDate and eventEndDate parameters from the path to the query parameters.
  2. jans-config-api/plugins/lock-plugin/src/main/java/io/jans/configapi/plugin/lock/rest/AuditResource.java:

    • The changes include the use of checkNotNull() and checkResourceNotNull() methods to handle cases where the event start or end date is null or cannot be parsed.
    • The event start and end dates are now passed as query parameters instead of as path parameters.
    • The code consistently uses the auditService.getDnForHealthEntry(), auditService.getDnForLogEntry(), and auditService.getDnForTelemetryEntry() methods to decode the event start and end dates.
  3. jans-config-api/docs/jans-config-api-swagger.yaml:

    • The changes update the whitePagesCanView, userCanView, userCanEdit, adminCanView, and adminCanEdit properties of the JansAttribute schema, which control the visibility and editability of attributes in the application.

Code Analysis

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

Analyzer Findings
Authn/Authz Analyzer 3 findings

Riskiness

🟢 Risk threshold not exceeded.

View PR in the DryRun Dashboard.

Copy link

DryRun Security Summary

We are currently analyzing this pull request to produce a meaningful summary.

Code Analysis

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

Analyzer Findings
Authn/Authz Analyzer 3 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 Sep 12, 2024
Copy link

sonarcloud bot commented Sep 12, 2024

@yuriyz yuriyz enabled auto-merge (squash) September 12, 2024 10:04
@yuriyz yuriyz merged commit 404b5bb into main Sep 12, 2024
1 of 2 checks passed
@yuriyz yuriyz deleted the jans-config-fix branch September 12, 2024 10:04
Copy link

sonarcloud bot commented Sep 12, 2024

yuriyz pushed a commit that referenced this pull request Nov 7, 2024
…9473)

* fix(config-api): asset mgt endpoint fixes

Signed-off-by: pujavs <pujas.works@gmail.com>

* feat(config-api): asset upload mgt ehancement and fido

Signed-off-by: pujavs <pujas.works@gmail.com>

* feat(config-api): asset upload mgt ehancement and fido

Signed-off-by: pujavs <pujas.works@gmail.com>

* feat(config-api): asset upload mgt ehancement and fido

Signed-off-by: pujavs <pujas.works@gmail.com>

* fix(config-api): asset upload

Signed-off-by: pujavs <pujas.works@gmail.com>

* fix(config-api): lock review comments

Signed-off-by: pujavs <pujas.works@gmail.com>

* feat(config-api): lock code review comments

Signed-off-by: pujavs <pujas.works@gmail.com>

* feat(config-api): lock master renamed to lock server

Signed-off-by: pujavs <pujas.works@gmail.com>

* feat(config-api): lock master renamed to lock server

Signed-off-by: pujavs <pujas.works@gmail.com>

* feat(config-api): lock master renamed to lock server

Signed-off-by: pujavs <pujas.works@gmail.com>

* feat(config-api): lock master renamed to lock server

Signed-off-by: pujavs <pujas.works@gmail.com>

* feat(config-api): fido2 delete functionality

Signed-off-by: pujavs <pujas.works@gmail.com>

* fix(config-api): acr validation

Signed-off-by: pujavs <pujas.works@gmail.com>

* feat(config-api): doc(config-api): IDP schema attribute descriptions #9187

Signed-off-by: pujavs <pujas.works@gmail.com>

* feat(config-api): sync with main

Signed-off-by: pujavs <pujas.works@gmail.com>

* feat(config-api): uploading assets via API generates 2 entries #9178

Signed-off-by: pujavs <pujas.works@gmail.com>

* feat(config-api): asset mgt, fido and IDP changes

Signed-off-by: pujavs <pujas.works@gmail.com>

* feat(config-api): fido2 device endpoint

Signed-off-by: pujavs <pujas.works@gmail.com>

* feat(config-api): fido2 endpoint

Signed-off-by: pujavs <pujas.works@gmail.com>

* feat(config-api): fido2 endpoint

Signed-off-by: pujavs <pujas.works@gmail.com>

* feat(config-api): sync with main

Signed-off-by: pujavs <pujas.works@gmail.com>

* feat(config-api): sync with main

Signed-off-by: pujavs <pujas.works@gmail.com>

* feat(config-api): sync with main

Signed-off-by: pujavs <pujas.works@gmail.com>

* feat(config-api): resolved sonar review issues

Signed-off-by: pujavs <pujas.works@gmail.com>

* feat(config-api): sonar review comment fix

Signed-off-by: pujavs <pujas.works@gmail.com>

* feat(config-api): swagger spec

Signed-off-by: pujavs <pujas.works@gmail.com>

* feat(config-api): saml config attribute description

Signed-off-by: pujavs <pujas.works@gmail.com>

* doc(config-api): added SAML attribute description

Signed-off-by: pujavs <pujas.works@gmail.com>

* doc(config-api): added SAML attribute description

Signed-off-by: pujavs <pujas.works@gmail.com>

* feat(config-api): sync with main

Signed-off-by: pujavs <pujas.works@gmail.com>

* fix(jans-lock): code review comment fix isssue#9305

Signed-off-by: pujavs <pujas.works@gmail.com>

* fix(jans-lock): code review comment fix isssue#9305

Signed-off-by: pujavs <pujas.works@gmail.com>

* feat(config-api): lock review point

Signed-off-by: pujavs <pujas.works@gmail.com>

* fix(lock): code review comment

Signed-off-by: pujavs <pujas.works@gmail.com>

* fix(lock): code review comment

Signed-off-by: pujavs <pujas.works@gmail.com>

* fix(config-api): sync with main

Signed-off-by: pujavs <pujas.works@gmail.com>

* feat(config-api): lock endpoint fixes and SAML IDP NPE

Signed-off-by: pujavs <pujas.works@gmail.com>

* feat(config-api): asset enhancement

Signed-off-by: pujavs <pujas.works@gmail.com>

* feat(config-api): implement timer for asset mgt to fetch and deploy assets forconfig-api #9403

Signed-off-by: pujavs <pujas.works@gmail.com>

* fix(config-api): scope validation issue #9426

Signed-off-by: pujavs <pujas.works@gmail.com>

* fix(config-api): asset delete error fix

Signed-off-by: pujavs <pujas.works@gmail.com>

* feat(config-api): sysnc with main

Signed-off-by: pujavs <pujas.works@gmail.com>

* fix(config-ap): lock audit endpoint parameter declaration error#9460

Signed-off-by: pujavs <pujas.works@gmail.com>

---------

Signed-off-by: pujavs <pujas.works@gmail.com>
Former-commit-id: 404b5bb
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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
Development

Successfully merging this pull request may close these issues.

fix(config-api): lock audit endpoint parameter declaration error
4 participants