Skip to content
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

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

Merged
merged 11 commits into from
Mar 25, 2022

Conversation

frital
Copy link
Contributor

@frital frital commented Apr 18, 2021

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
Copy link
Contributor Author

frital commented May 1, 2021

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
Copy link
Contributor Author

frital commented Jul 13, 2021

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
Copy link
Owner

@frital hi there. Could you please include tests?

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

@OfficialBAMM
Copy link
Contributor

I'd like to have this merged as well.

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

@frital
Copy link
Contributor Author

frital commented Mar 21, 2022

Hi @OfficialBAMM !

This would be nice!! Thx in advance!

@robsontenorio
Copy link
Owner

robsontenorio commented Mar 21, 2022

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

@OfficialBAMM
Copy link
Contributor

Created the PR. Let me know what you think!

@robsontenorio
Copy link
Owner

@OfficialBAMM I think you forgot to open PR

@OfficialBAMM
Copy link
Contributor

@robsontenorio I opened an PR to this PR.

See: frital#1

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

@frital
Copy link
Contributor Author

frital commented Mar 23, 2022

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

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

@leidison
Copy link

@frital,

I was already about to implement an egalitarian solution.

Congratulations for the initiative. I need this feature.

@robsontenorio robsontenorio merged commit fe6b9c5 into robsontenorio:master Mar 25, 2022
@robsontenorio
Copy link
Owner

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

@OfficialBAMM
Copy link
Contributor

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants