Release 0.113.2 #2576
Release 0.113.2 #2576
Conversation
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
|
| GitGuardian id | GitGuardian status | Secret | Commit | Filename | |
|---|---|---|---|---|---|
| 13777604 | Triggered | Generic High Entropy Secret | 53b1880 | config/keycloak/realms/default-realm.json | View secret |
| 13777605 | Triggered | Generic High Entropy Secret | 53b1880 | config/keycloak/realms/default-realm.json | View secret |
| 13777606 | Triggered | Generic Private Key | 53b1880 | config/keycloak/tls/tls.key | View secret |
| 13777607 | Triggered | Generic High Entropy Secret | 53b1880 | config/keycloak/realms/default-realm.json | View secret |
| 13777608 | Triggered | Generic High Entropy Secret | 53b1880 | config/keycloak/realms/default-realm.json | View secret |
| 13777609 | Triggered | Generic Password | 53b1880 | config/keycloak/realms/default-realm.json | View secret |
| 13777610 | Triggered | Generic High Entropy Secret | 53b1880 | config/keycloak/realms/default-realm.json | View secret |
| 10259317 | Triggered | Generic Password | 53b1880 | docker-compose.yml | View secret |
🛠 Guidelines to remediate hardcoded secrets
- Understand the implications of revoking this secret by investigating where it is used in your code.
- Replace and store your secrets safely. Learn here the best practices.
- Revoke and rotate these secrets.
- If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.
To avoid such incidents in the future consider
- following these best practices for managing and storing secrets including API keys and other credentials
- install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.
🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hello @odlbot, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
Summary of Changes
This pull request updates the MITx Online platform to release version 0.113.2. It incorporates several key changes, including seeding data for testing courses, automatically creating a cache table, and adding OIDC login via social-auth. The changes also include updates to the Dockerfile, pre-commit configurations, and settings to accommodate the new Keycloak integration.
Highlights
- Keycloak Integration: Adds support for OIDC login via social-auth, including a sample Keycloak instance for local development and testing. This involves new settings, authentication backends, and pipeline configurations.
- Course Data Seeding: Includes seed data for testing courses locally and in the RC environment, making it easier to develop and test course-related features.
- Cache Table Automation: Automatically creates a cache table, likely to improve performance and efficiency of data retrieval.
- Dockerfile Updates: Updates the Dockerfile to include Keycloak configuration files and SSL certificates, streamlining the setup process for Keycloak integration.
- Pre-commit Configuration: Updates pre-commit configuration to exclude Keycloak configuration files from certain checks, preventing false positives.
Changelog
Click here to see the changelog
- .env.example
- Added environment variables for Keycloak service configuration, including hostname, ports, admin credentials, and keystore password.
- .pre-commit-config.yaml
- Excluded Keycloak configuration files from pre-commit checks to avoid unnecessary warnings.
- .secrets.baseline
- Updated to exclude Keycloak configuration files from secret scanning.
- Updated the line number for a hashed secret in
main/settings.pyfrom 959 to 1014. - Updated the
generated_attimestamp.
- Dockerfile
- Added instructions to copy
mitol_*.gzfiles to the /app directory within the container.
- Added instructions to copy
- README-keycloak.md
- Added a new README file with instructions on how to integrate MITx Online with Keycloak, including default settings, SSL certificate setup, realm configuration, and troubleshooting tips.
- README.md
- Added a section on Keycloak Integration, referencing the new
README-keycloak.mdfile.
- Added a section on Keycloak Integration, referencing the new
- RELEASE.rst
- Added release notes for version 0.113.2, listing the key changes included in this release.
- app.json
- Added new environment variables to
app.jsonfor Keycloak configuration, OIDC settings, and exposing OIDC login functionality.
- Added new environment variables to
- authentication/backends/ol_open_id_connect.py
- Created a custom wrapper class
OlOpenIdConnectAuthfor Keycloak authentication, extendingOpenIdConnectAuthto include user details retrieval and email validation.
- Created a custom wrapper class
- authentication/pipeline/user.py
- Modified the authentication pipeline to include steps for handling OIDC logins, including user creation and blocked user checks.
- Added
forbid_hijackto prevent admin users from logging in while hijacking another user. - Modified
get_usernameto handle OIDC logins.
- authentication/pipeline/user_test.py
- Added tests for the
create_ol_oidc_userfunction to ensure correct user creation and handling of OIDC logins.
- Added tests for the
- authentication/views.py
- Modified the logout view to redirect users to the Keycloak logout URL after logging out from MITx Online.
- Added import for
OlOpenIdConnectAuth.
- config/db/init-keycloak.sql
- Added SQL script to create the
keycloakdatabase and grant privileges to thepostgresuser.
- Added SQL script to create the
- config/keycloak/realms/default-realm.json
- Added a default realm configuration file for Keycloak, including users, roles, and client settings.
- config/keycloak/tls/README.md
- Added a README file with instructions on how to configure TLS for Keycloak, including generating self-signed certificates.
- config/keycloak/tls/tls.crt
- Added a default TLS certificate file.
- config/keycloak/tls/tls.crt.default
- Added a default TLS certificate file.
- config/keycloak/tls/tls.key
- Added a default TLS key file.
- config/keycloak/tls/tls.key.default
- Added a default TLS key file.
- courses/management/commands/populate_course_data.py
- Added a management command to seed the database with course and course run data for testing purposes.
- courses/management/courses.json
- Added a JSON file containing course and course run data for testing purposes.
- courses/management/utils.py
- Added a utility function to load JSON data from a file.
- docker-compose-keycloak-override.yml
- Added a docker-compose override file to add an alias for the Keycloak hostname to
host-gateway.
- Added a docker-compose override file to add an alias for the Keycloak hostname to
- docker-compose.yml
- Added a Keycloak service configuration to the docker-compose file, including environment variables, ports, volumes, and command.
- ecommerce/api.py
- Removed an extra newline character.
- frontend/public/src/components/AnonymousMenu.js
- Modified the AnonymousMenu component to conditionally render login and create account links based on the
oidc_login_urlsetting.
- Modified the AnonymousMenu component to conditionally render login and create account links based on the
- frontend/public/src/components/AnonymousMenu_test.js
- Added tests for the AnonymousMenu component to ensure correct rendering of login and create account links based on the
oidc_login_urlsetting.
- Added tests for the AnonymousMenu component to ensure correct rendering of login and create account links based on the
- frontend/public/src/flow/declarations.js
- Added a declaration for the
oidc_login_urlsetting.
- Added a declaration for the
- frontend/public/src/lib/urls.js
- Added
emailroute to login urls.
- Added
- main/settings.py
- Updated the version number to 0.113.2.
- Added settings for Keycloak integration, including OIDC endpoints, client IDs, and secrets.
- Added
EXPOSE_OIDC_LOGINsetting to control the visibility of OIDC login functionality. - Added
KEYCLOAK_BASE_URLandKEYCLOAK_REALM_NAMEsettings for logout redirection. - Added
OlOpenIdConnectAuthtoAUTHENTICATION_BACKENDS. - Added
global_idtoSOCIAL_AUTH_IMMUTABLE_USER_FIELDS.
- main/utils.py
- Modified
get_js_settingsto include the OIDC login URL ifEXPOSE_OIDC_LOGINis enabled.
- Modified
- main/utils_test.py
- Added tests for
get_js_settingsto ensure correct rendering of OIDC login URL based on theEXPOSE_OIDC_LOGINsetting.
- Added tests for
- scripts/run-django-dev.sh
- Added
createcachetablecommand to the script.
- Added
- users/factories.py
- Added
global_idto UserFactory.
- Added
- users/management/commands/promote_user.py
- Added a management command to promote or demote users to staff or superuser status.
- users/management/tests/promote_user_test.py
- Added tests for the
promote_usermanagement command.
- Added tests for the
- users/models.py
- Added
global_idfield to the User model to store the SSO ID.
- Added
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Trivia time!
What is the default port used by Keycloak for HTTPS?
Click here for the answer
The default port used by Keycloak for HTTPS is 8443.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request introduces Keycloak integration, which is a significant feature. The changes span multiple files, including .env.example, Dockerfile, README files, and Python code. The Keycloak integration seems well-structured, but there are some security concerns and areas for improvement, particularly around input sanitization and documentation clarity.
Summary of Findings
- Sensitive information in .env.example: The
.env.examplefile contains a default password (supersecret123456789) forKEYCLOAK_SVC_KEYSTORE_PASSWORD. This poses a security risk as it could be used in a production environment if not changed. - Missing input sanitization: The code related to Keycloak integration does not appear to include input sanitization. This could lead to vulnerabilities such as injection attacks.
Merge Readiness
The pull request introduces significant functionality with the Keycloak integration. While the code appears to be well-structured, the presence of a default password in .env.example and the lack of input sanitization are critical security concerns that must be addressed before merging. Additionally, the documentation could be improved for clarity. Therefore, I recommend that this pull request not be merged until these issues are resolved. I am unable to approve this pull request, and other reviewers should carefully examine the changes before considering approval.
| @@ -16,3 +16,10 @@ POSTHOG_PROJECT_API_KEY= | |||
| POSTHOG_API_HOST=https://app.posthog.com/ | |||
| HUBSPOT_HOME_PAGE_FORM_GUID= | |||
| HUBSPOT_PORTAL_ID= | |||
|
|
|||
| KEYCLOAK_SVC_KEYSTORE_PASSWORD=supersecret123456789 | |||
There was a problem hiding this comment.
The KEYCLOAK_SVC_KEYSTORE_PASSWORD is set to supersecret123456789 by default. This is a major security risk, as anyone deploying the application without changing this password will be vulnerable. Consider removing the default value or generating a random one during deployment. It is recommended to not have a default value at all, forcing the user to set one.
KEYCLOAK_SVC_KEYSTORE_PASSWORD=
| def _keycloak_logout_url(self, user): | ||
| """ | ||
| Return the OpenID Connect logout URL for a user based on | ||
| their SocialAuth record's id_token and the currently | ||
| configured Keycloak environment variables. | ||
|
|
||
| Args: | ||
| user (User): User model record associated with the SocialAuth record. | ||
|
|
||
| Returns: | ||
| string: The URL to redirect the user to in order to logout. | ||
| """ | ||
| strategy = load_strategy() | ||
| storage = strategy.storage | ||
| user_social_auth_record = storage.user.get_social_auth_for_user( | ||
| user, provider=OlOpenIdConnectAuth.name | ||
| ).first() | ||
|
|
||
| if not user_social_auth_record: | ||
| return None | ||
|
|
||
| id_token = user_social_auth_record.extra_data.get("id_token") | ||
| qs = urlencode( | ||
| { | ||
| "id_token_hint": id_token, | ||
| "post_logout_redirect_uri": self.request.build_absolute_uri( | ||
| settings.LOGOUT_REDIRECT_URL | ||
| ), | ||
| } | ||
| ) | ||
|
|
||
| return ( | ||
| f"{settings.KEYCLOAK_BASE_URL}/realms/" | ||
| f"{settings.KEYCLOAK_REALM_NAME}/protocol/openid-connect/logout" | ||
| f"?{qs}" | ||
| ) |
There was a problem hiding this comment.
The _keycloak_logout_url method constructs a URL using settings.KEYCLOAK_BASE_URL and settings.KEYCLOAK_REALM_NAME. Ensure that these settings are properly validated and that the constructed URL is safe from injection attacks. Consider using urllib.parse.urljoin to construct the URL safely.
| @@ -43,6 +43,8 @@ repos: | |||
| - compliance/test_data/cybersource/ | |||
| - --exclude-files | |||
| - ".*_test.js" | |||
| - --exclude-files | |||
| - "config/keycloak/*" | |||
There was a problem hiding this comment.
Excluding config/keycloak/* from pre-commit hooks and secrets scanning is a good idea to prevent secrets from being accidentally committed. However, ensure that this exclusion is intentional and doesn't bypass necessary security checks for other important files in the config directory.
| @@ -119,7 +119,8 @@ | |||
| "poetry.lock", | |||
| "yarn.lock", | |||
| "compliance/test_data/cybersource/", | |||
| ".*_test.js" | |||
| ".*_test.js", | |||
| "config/keycloak/*" | |||
There was a problem hiding this comment.
Excluding config/keycloak/* from secrets scanning is a good idea to prevent secrets from being accidentally committed. However, ensure that this exclusion is intentional and doesn't bypass necessary security checks for other important files in the config directory.
| if retval.get("is_new"): | ||
| retval["user"].is_active = True | ||
| retval["user"].save() |
There was a problem hiding this comment.
In create_ol_oidc_user, the is_active flag is set to True for new users. Ensure that this is the desired behavior and that there are no security implications. Consider adding a comment explaining why this is being done.
odlbot
added
waiting for checkboxes
all checkboxes checked
and removed
deploying to rc
waiting for checkboxes
labels
odlbot
added
deploying to prod
deployed to prod
and removed
all checkboxes checked
deploying to prod
labels
annagav
Chris Chudzicki
James Kachel