-
Notifications
You must be signed in to change notification settings - Fork 0
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
Closes #16 Implement o auth Google #61
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As We do not have existing data I would sugest deleting everything from Migrations directory and creating a single Init
migration.
@Sojusan I think I'm done with the initial review of the code. Sorry for the amount of things pointed out. Feel free to point out disagree with the points 😉 Now going to the functionality: However I'm very concerned about one thing. It seams that I can login using Keycloak and frontend recognizes it, but the backend does not. I have run the application using docker compose with both the env file and appsettings file You have provided. This are the three issues I have encoutered:
On the brightside:
|
I have created a separate issue #62 in order to investigate how to create our own template for Keycloak and then to create it and add our UI style to it. Will work on it after this PR.
Deleted. |
Quality Gate passed for 'ksummarized_ksummarized.com_frontend'Issues Measures |
Quality Gate failed for 'ksummarized_ksummarized.com_backend'Failed conditions |
Yeah, there was an error in the configuration for our realm in the Keycloak. I didn't set explicitly the RSA Private and Public keys in the export-realm.json file. This way when you started a fresh instance of the Keycloak on your machine the Keycloak just generated a new pair of these keys, so the backend was dependent on the old one. In order if my fix is working correctly now, please take the newest environment settings (both appsettings and .env) from our docs, prune containers, and volumes, then start once again. The Keycloak should now take keys from the environment variable and validation on the backend site should work correctly.
It is related to the previous error. If the token is detected as invalid then the backend refuses that connection without creating a user.
I'm not sure what approach should be there. We changed the authenticating service from our backend to the Keycloak service so I think it is correct to see the frontend after successful authentication in the Keycloak service. The backend is now validating tokens just for security and I think it shouldn't block users from visiting the website when it has access to per the Keycloak service. I think it is enough to just prompt an error when we will be working on the error handling on the frontend. |
The changes look great, I wiil approve 😄 Have You checked this failing pipeline do We still need to fix it or is it a false positvie? EDIT: I have read the errors, We can safely ignore them 👍🏼 |
External OAuth providers added: Google, Twitter/X, Facebook, GitHub.
It is done via the Keycloak service. More details can be checked in our docs repo.
I have changed the way we handle
.env
andappsettings.Development.json
. They are now not included by default in the repo as they will contain sensitive data like OAuth providers secrets.ℹ️ I did my changes based on your (@mtracewicz ) branch for this feature, so it includes some of your improvements from there, e.g., Serilog implementation.