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

docs: default acr script validation #8715

Merged
merged 18 commits into from
Jun 28, 2024
Merged

docs: default acr script validation #8715

merged 18 commits into from
Jun 28, 2024

Conversation

pujavs
Copy link
Contributor

@pujavs pujavs commented Jun 14, 2024

Prepare


Description

Target issue

closes #8707

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)

pujavs added 10 commits April 8, 2024 21:53
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>
Copy link

dryrunsecurity bot commented Jun 14, 2024

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

DryRun Security Status Findings
Configured Codepaths Analyzer 0 findings
IDOR Analyzer 0 findings
Secrets Analyzer 0 findings
Authn/Authz Analyzer 0 findings
Server-Side Request Forgery Analyzer 0 findings
SQL Injection Analyzer 0 findings
Sensitive Files 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 code changes in this pull request focus on improving the configuration and management of the default authentication method and custom scripts in the Janssen Server. From an application security perspective, the changes introduce several security-relevant features, such as validation of the availability and active status of the authentication method, clear error handling, and documented authentication method selection flow. Additionally, the ability to manage custom scripts through a command-line interface (CLI) provides flexibility, but also introduces potential security risks that should be carefully considered.

The key security considerations include: ensuring that only valid and active authentication methods are used as the default, reviewing the code of custom scripts to mitigate vulnerabilities, carefully managing the configuration properties of custom scripts to prevent misconfigurations or unintended access to sensitive data, and implementing strict access controls and secure deployment practices to prevent unauthorized modifications to the custom scripts. Overall, the changes appear to be focused on improving the security and reliability of the Janssen Server's authentication and custom script management functionality.

Files Changed:

  1. docs/admin/config-guide/default-authentication-method-config.md:

    • This file covers the configuration of the default authentication method in the Janssen Server, including the availability validation, error handling, and authentication method selection flow.
    • The changes highlight the security-focused aspects of the default authentication method configuration, such as the availability check and the clear error messaging.
    • The documented authentication method selection flow helps in understanding the server's behavior and ensures a well-defined and consistent authentication process.
  2. docs/admin/config-guide/custom-scripts-config.md:

    • This file covers the management of custom scripts in the Janssen application, including the addition, updating, retrieval, and deletion of scripts through a command-line interface (CLI).
    • The security considerations include the potential impact of modifying or removing default authentication scripts, the risks associated with the supported scripting languages (Python and JavaScript), the various script types and their security implications, the management of configuration properties, and the potential for privilege escalation through the CLI.
    • Administrators should carefully review and validate the custom scripts, ensure appropriate security measures are in place, and implement strict access controls and secure deployment practices.

Powered by DryRun Security

@mo-auto mo-auto added the area-documentation Documentation needs to change as part of issue or PR label Jun 14, 2024
Signed-off-by: pujavs <pujas.works@gmail.com>
Signed-off-by: pujavs <pujas.works@gmail.com>
mo-auto
mo-auto previously approved these changes Jun 14, 2024
@mo-auto mo-auto enabled auto-merge (squash) June 14, 2024 14:10
Signed-off-by: pujavs <pujas.works@gmail.com>
mo-auto
mo-auto previously approved these changes Jun 17, 2024

!!! Note

If a custom script that is set as *Default authentication method* is disabled or deleted then the default authentication value will be removed.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pujavs if it is removed, what will be the fallback default authentication? Can AS live without any default authn method defined?

Copy link
Contributor Author

@pujavs pujavs Jun 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yuriyz, my understanding that AS can live without explicit setting of default auth method, request your confirmation and advice.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, if nothing works, and no scripts then AS will fallback to "simple_password_auth".

Signed-off-by: ossdhaval <343411+ossdhaval@users.noreply.github.com>
ossdhaval
ossdhaval previously approved these changes Jun 19, 2024
Signed-off-by: ossdhaval <343411+ossdhaval@users.noreply.github.com>
@mo-auto mo-auto merged commit a394f11 into main Jun 28, 2024
8 of 9 checks passed
@mo-auto mo-auto deleted the jans-docs branch June 28, 2024 14:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-documentation Documentation needs to change as part of issue or PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

docs: validation and changed for default acr script
4 participants