search

sidferreira / aor-firebase-client

Firebase client for Admin on Rest
MIT License
57 stars 29 forks source link

Check auth status before bind on firebase db change #22

Closed kidphys closed 6 years ago

kidphys commented 7 years ago

@sidferreira Could you help review the pull request?

shrmnk commented 7 years ago

I saw that you added support for sorting. Did you include array-sort in the dependencies?

grahamlyus commented 7 years ago

@kidphys Seems to me like the filtering and sorting should be separate PRs - I don't use the auth client so can't verify that fix

sidferreira commented 7 years ago

@grahamlyus I'll take a look on the auth thing this week. Might also work on the upload too

sidferreira commented 7 years ago

@kidphys @shrmnk Guys, can you help splitting this PR?

shrmnk commented 7 years ago

@sidferreira Alright I've created a PR #23 just for the sorting. @kidphys hope you don't mind! I had this functionality on my production site so was ready to put it up.

grahamlyus commented 7 years ago

Good point - I haven't used any public resources in my little project, but I could see that being an issue.

grahamlyus commented 7 years ago

@sidferreira @shrmnk @kidphys

Upon further reflection, it seems unlikely that an unauthenticated user would have access to the admin, regardless of the actual database permissions, so maybe it's a moot point?

shrmnk commented 7 years ago

Agreed on that. I think it’s unlikely a site will have a mix of public/unauthenticated admin pages with protected ones.

sidferreira commented 7 years ago

@grahamlyus @shrmnk true... but it will force the user always setup the auth part. Start and admin without auth to add it later will not be possible, as it will wait for the usetAuth callback.

We can just add a big IF... We add the option doNotWaitForUserAuth (or something) and if it is set it will not wait for the callback. Default behavior is to wait for it for safety reasons.

sidferreira commented 6 years ago

@kidphys @grahamlyus @shrmnk I'm about to release v0.2.0 which has it fixed. I'm finishing the tests. Feel free to use/test and help me out!