karlomikus / bar-assistant

Bar assistant is a all-in-one solution for managing your home bar
https://barassistant.app
MIT License
587 stars 22 forks source link

Draft: OIDC Authentication Implementation #355

Closed IamTaoChen closed 1 week ago

IamTaoChen commented 2 weeks ago

Description

This PR introduces three new endpoints to support OIDC (OpenID Connect) authentication:

Flow

  1. Request OIDC Login:

The user initiates the OIDC login by sending a POST request to /auth/oidc. This request accepts two optional parameters:

The response includes { auth_url, code }, where:

  1. Redirect to IdP for Authentication: The user is redirected to the auth_url provided in the first step, where they complete the login process at the IdP. Upon successful authentication, the IdP redirects the user back to /auth/oidc/callback.

  2. Token Exchange: The user then uses the code received in the initial step to exchange it for an authentication token by sending a POST request to /auth/oidc/token.

IamTaoChen commented 2 weeks ago

I try to test manually, it works with Keycloak. Next, I'll try to add to the frontend and test.

IamTaoChen commented 2 weeks ago

frontend basic example: https://github.com/karlomikus/vue-salt-rim/pull/250

karlomikus commented 1 week ago

Hi, I appreciate the effort you've put into it. However, after reviewing the changes, I believe this code does not meet the standards I maintain in this project.

Some quick observations: missing unit tests, missing integration tests (which are crucial for the type of functionality this code introduces), not conforming to code style, breaking single responsibility pattern in controller, etc.

Also, since this is Laravel project, I would look into integration existing ecosystem packages like https://github.com/laravel/passport

I appreciate that you've tested the code, but merging it in its current state would introduce significant maintenance challenges in the future.

You can try to resolve the issues mentioned above and resubmit if you'd like. Thanks!