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

fix: Set authenticated: true after successful authentication #1367

Merged
merged 1 commit into from
May 25, 2019

Conversation

KidkArolis
Copy link
Contributor

Summary

Fixes #1360

@daffl
Copy link
Member

daffl commented May 25, 2019

Good call. I removed it because it seemed redundant but with #1317 bringing back the param it makes sense to set it.

@daffl daffl merged commit 9918cff into feathersjs:master May 25, 2019
@KidkArolis
Copy link
Contributor Author

One alternative thought..

When i first wrote some feathers tests, it seemed a little superfluous that i had to pass both { user, authenticated }.

And so i think it was interesting that you were trying to remove the need for that param, by hiding it more behind an internal symbol.

But what if presence of user object would be enough to treat requests as authenticated. So instead of using/checking authenticated we just have { user }.

@KidkArolis KidkArolis deleted the set-authenticated branch May 30, 2019 12:10
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.

[Feathers 4.0.0-pre.2] Authenticated: true is not added to context.params
2 participants