search

robsontenorio / laravel-keycloak-guard

🔑 Simple Keycloak Guard for Laravel
MIT License
434 stars 141 forks source link

Feature: allow create / update users on the API users table with autenticated token #45

Closed frital closed 2 years ago

frital commented 3 years ago

This PR aims to, as an option, allow the API to create / update the authenticated user in the local users table. To achieve this, its possible to inform a custom method on a custom UserProvider, that will be called instead retrieveByCredentials and will receive the complete decoded token as parameter, not just the credentials.

frital commented 3 years ago

Hi @robsontenorio ! Any special reason to not accept this PR? It is out of scope or something missing? Pls let me know! Thx in advance!

frital commented 3 years ago

Alo @robsontenorio ! Tudo bem? Alguma razao específica pra nao querer aceitar o PR? Ou só correria mesmo? :) Muito obrigado por antecipacao, e no seu aguardo!! Abracos!!

robsontenorio commented 3 years ago

@frital hi there. Could you please include tests?

I am not sure about the flow. With tests we could review it.

OfficialBAMM commented 2 years ago

I'd like to have this merged as well.

Can I create the tests for you so you can merge the request?

frital commented 2 years ago

Hi @OfficialBAMM !

This would be nice!! Thx in advance!

robsontenorio commented 2 years ago

@OfficialBAMM for sure! We need tests to keep consistence through all features :)

OfficialBAMM commented 2 years ago

Created the PR. Let me know what you think!

robsontenorio commented 2 years ago

@OfficialBAMM I think you forgot to open PR

OfficialBAMM commented 2 years ago

@robsontenorio I opened an PR to this PR.

See: https://github.com/frital/laravel-keycloak-guard/pull/1

Atleast I think that how this should be done, but I might be wrong.

frital commented 2 years ago

Thx @OfficialBAMM !! I just merged you PR!!

I hope that now @robsontenorio will be able to accept the PR. =) Thx guys!!

leidison commented 2 years ago

@frital,

I was already about to implement an egalitarian solution.

Congratulations for the initiative. I need this feature.

robsontenorio commented 2 years ago

Could you guys test master branch with this feature DISABLED if everything is ok, before I release a tag ?

OfficialBAMM commented 2 years ago

Just tested it on my application with an empty string or removed at all. Both worked for me.

Given a random string gives a Call to undefined method App\\Providers\\KeycloakEloquentUserProvider::randomFunction(). This should probably give a better error. If you're interested I could make a PR for it.

Looking at the code, I see no reason why it shouldn't work for existing installations.