APISIX integration #2061
APISIX integration #2061
Conversation
|
| GitGuardian id | GitGuardian status | Secret | Commit | Filename | |
|---|---|---|---|---|---|
| 13777606 | Triggered | Generic Private Key | dab67da | config/keycloak/tls/tls.key.default | View secret |
| 13777608 | Triggered | Generic High Entropy Secret | dab67da | config/keycloak/realms/default-realm.json | View secret |
| 13777609 | Triggered | Generic Password | dab67da | config/keycloak/realms/default-realm.json | View secret |
| 13777610 | Triggered | Generic High Entropy Secret | dab67da | config/keycloak/realms/default-realm.json | 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.
main/settings.py
Outdated
| "authentication.pipeline.user.user_created_actions", | ||
| # redirect new users to onboarding | ||
| "authentication.pipeline.user.user_onboarding", | ||
| ) |
There was a problem hiding this comment.
This didn't seem to be getting used even without keycloak and apisix
There was a problem hiding this comment.
It's definitely being used, should be left in.
There was a problem hiding this comment.
OK, adding it back, must have misunderstood yesterday's slack chat, I thought apisix was supposed to be a replacement for the pipeline. Currently both the apisix middleware and the pipeline trigger profile creation for new users, should I remove from one or the other?
There was a problem hiding this comment.
PS the final pipeline step is supposed to redirect the user to "/onboarding" but that isn't currently happening on RC with new users I created there, even though MITOL_NEW_USER_LOGIN_URL=https://rc.learn.mit.edu/onboarding - should it be? Haven't tried it on production yet.
There was a problem hiding this comment.
LGTM 👍 was able to log in successfully via Keycloak. It also attached the keycloak user to the [email protected] superuser I created via the management command.
docker-compose.services.yml
Outdated
| profiles: | ||
| - keycloak | ||
| image: apache/apisix:latest | ||
| depends_on: | ||
| - keycloak |
There was a problem hiding this comment.
I would remove profiles and depends_on here, because otherwise I'm forced to spend resources running a duplicate keycloak instance I won't use if I'm not running the keycloak container defined above.
There was a problem hiding this comment.
Might it be useful to still keep the profiles part, but change it to its own value so that apisix (and keycloak) don't start automatically, only when explicitly requested? So:
COMPOSE_PROFILES=backend,frontend,keycloak,apisix starts everything
COMPOSE_PROFILES=backend,frontend,apisix starts apisix but not keycloak
etc
There was a problem hiding this comment.
I just made that change (removed depends_on, gave keycloak and apisix containers their own separate profiles)
There was a problem hiding this comment.
Simpler workflow seems to work pretty well. I have one nit regarding logging. (I am also only evaluating the APISIX/Keycloak integration parts of this.)
I had some issues getting to the onboarding until I figured out that the onboarding URL is configurable. May be a good idea to add that to the docs (or update the pipeline or default to use the app base URL) - it defaults to just /onboarding which ends up sending the user to the API service and results in an error.
main/middleware/apisix_user.py
Outdated
| """ | ||
| if request.META.get(self.header): | ||
| new_header = decode_apisix_headers(request, self.header) | ||
| log.error("FOUND APISIX HEADER: %s", new_header) |
There was a problem hiding this comment.
nit: this is logging as error
What are the relevant tickets?
Closes https://github.com/mitodl/hq/issues/6756
Related infrastructure PR: mitodl/ol-infrastructure#2983
Description (What does it do?)
How can this be tested?
Follow instructions in the new README. You should be able to log in and log out.