-
Notifications
You must be signed in to change notification settings - Fork 3
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
Add Ory Kratos Integration. #198
Conversation
- kratos database in same postgres server with backend database configuration is in the docker-compose.yaml - add auth controller for the kratos only if the kratos required flag is set to true - change in the user table schema, password and roles are nullable now - kratos_id addition and model for the same for databae insertion
config/kratos.go
Outdated
BaseURL string `envconfig:"SERVE_PUBLIC_BASE_URL"` | ||
UIUrl string `envconfig:"SELF_SERVICE_DEFAULT_BROWSER_RETURN_URL"` | ||
AdminUrl string `envconfig:"SERVE_ADMIN_BASE_URL"` | ||
PublicUrl string `envconfig:"SERVE_PUBLIC_BASE_URL"` |
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.
@Shaktizala What is different between BaseURL and PublicURL? Also keep consistency in naming. In BaseURL
, URL is in uppercase when in PublicUrl and AdminUrl it is in CamelCase
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.
Both are same, it's the kratos base url, but as kratos requires envs to be in specific format it is kept like, "SERVE_PUBLIC_BASE_URL".
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.
@Shaktizala If both are same then why are you using two variables for same env?
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.
My bad, it was there for previous env configuration, will remove it.
// 400: GenericResFailBadRequest | ||
// 500: GenericResError | ||
func (ctrl *AuthController) DoKratosAuth(c *fiber.Ctx) error { | ||
kratosID := c.Locals(constants.KratosID) |
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.
@Shaktizala What is the purpose of using c.Locals
?
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.
It's used to fetch kratosID into the next handle. It means we can add it to locals from here and fetch it from the next handler.
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.
@Shaktizala Is it middleware? Middleware shouldn't be in controller
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.
No, it's not middleware. used to pass an ID from middleware to next handler.
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.
I mean, is DoKratosAuth
middleware?
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.
No, it's not middleware. It's an endpoint on which kratos will redirect after successful login/registration.
pkg/kratos/readme.md
Outdated
route.Get("/\<end-point\>", middlewares.Authenticated, authController.DoKratosAuth) | ||
|
||
## How Kratos Integration Works? | ||
You have to provide Kratos with your UI endpoint URLs, Kratos will redirect to those endpoints by initializing the flow. |
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.
@Shaktizala What frontend has to sent? Should it work with ajax request? Does it need to follow any xml/json format? Or it is form?
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.
Yes, it is in form.
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.
Then please document that or add link of external doc
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.
okay will update it.
Co-authored-by: Munir Khakhi <6319375+munir131@users.noreply.github.com> Signed-off-by: Shaktiraj Zala <123356347+Shaktizala@users.noreply.github.com>
Co-authored-by: Munir Khakhi <6319375+munir131@users.noreply.github.com> Signed-off-by: Shaktiraj Zala <123356347+Shaktizala@users.noreply.github.com>
c.Redirect(ctrl.config.Kratos.UIUrl) | ||
|
||
c.Locals(constants.KratosUserDetails, user) | ||
c.Next() |
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.
@Shaktizala What is purpose of it? L154 will already redirect on UI
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.
sorry, forgot to remove it, it was there when I was not doing redirection. will update it.
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.
- Define step pkg/kratos/oidc then run base64 -w 0 google.schema.jsonnet
- Mention the steps for OAuth 2.0 client IDs and redirect URLs set.
- Start service in on go, like Kratos up command then migrate up then api run etc.
- Info: If the password field is not required, then please remove it.
@pratikbgit Password field is required, when kratos is not enabled We have to use authentication using password so at that time we need to store password. I am not getting the 3rd statement, can you please elaborate. CC: @munir131 |
While kratos service start before that migrate and api service should start not manually. |
@pratikbgit
|
… the backend server all together. - update docs accordingly
@Shaktizala Please resolve conflicts |
Signed-off-by: Shaktiraj Zala <123356347+Shaktizala@users.noreply.github.com>
Closes #193
Changes proposed
Check List (Check all the applicable boxes)