Gizra / elm-restful

Elm utilities useful for working with a RESTful backend (especially Drupal)
MIT License
3 stars 0 forks source link

Expose `encodeCredentials` #14

Closed amitaibu closed 6 years ago

amitaibu commented 6 years ago

I have a user case, where I get the access token/ user myself (e.g. after registration), so I'd like to save the credentials.

Or maybe we can have a utility to mimic

cmd = loginConfig.cacheCredentials credentials.backendUrl (Restful.Login.encodeCredentials loginConfig credentials

rgrempel commented 6 years ago

Ah, yes, I see what you mean -- there isn't a way to do that at the moment.

I think your second idea is better, because it would probably be nice to leave encodeCredentials as an implementation detail. Perhaps what we need is a variation of loggedIn that caches the supplied credentials ... so, it would need to be a Msg. I'll think about what the best way to structure that would be.

amitaibu commented 6 years ago

Indeed, probably something like Restful.Login.CacheCredentials loginConfig credentials is the right approach.

amitaibu commented 6 years ago

@rgrempel it would be great if you could have a look at this, as others are starting to use my hack :)

rgrempel commented 6 years ago

I've taken another look at this, and I think there are several sensible alternatives.

One would be to focus on the access token. If you get an access token independently, we can fill in the rest of the credentials ourselves -- it would look something like this:

{-| Given an access token you have obtained, try using it to obtain the `Credentials` from 
the backend, and cache them if successful.

This is an alternative to `checkCachedCredentials` for cases in which what you have is
not an encoding of the full `Credentials`, but just an access token.
-}
checkAccessToken : Config anonymousData user authenticatedData msg -> BackendUrl -> String -> ( UserAndData anonymousData user authenticatedData, Cmd msg )

So, basically this would be like checkCachedCredentials, but you'd start the process with just an access token. The backend would be contacted to get the rest of the credentials -- if this succeeds, you'd have an AuthenticatedUser (and the credentials would be cached). If it fails, you'd have an AnonymousUser.

Now, this approach actually contacts the backend in order to (a) verify that the access token is actually valid and (b) obtain the User. What about cases where you have the User as well? I suppose that in that case we could avoid contacting the backend at all, since we know everything we need. However, it might be just as well to contact the backend anyway -- it won't take long! So, checkAccessToken might look like this instead of the above:

{-| An alternative to `checkCachedCredentials` for cases where you don't have the 
cached credentials, but you do have an access token, a `user`, or  both.

- If you supply the access token, we will contact the backend to verify that it is valid.
   - If that succeeds, we will be in `AuthenticatedUser` state, with credentials reflecting
      the supplied access token, and the `user` returned by the backend.
   - If that fails, then our state will depend on whether you supplied a `user` or not.
      - If you supplied a `user`, we will be in `AuthenticatedUser` state (with that `user`), 
         but our `relogin` field will indicate the error checking the access token.
      - If you did not supply a `user`, we will be in `AnonymousUser` state, with the 
         `progress` field indicating the error checking the access token.
- If you don't supply an access token, then our state depends on whether you provide a
   `user` or not.
   - If you provide a `user`, we will be in `AuthenticatedUser` state with that user, but our
      `relogin` field will indicate that the access token was rejected.
   - If you didn't provide a `user`, we will be in `AnonymousUser` state.
-}
checkAccessToken : Config anonymousData user authenticatedData msg -> BackendUrl -> Maybe String -> Maybe user -> ( UserAndData anonymousData user authenticatedData, Cmd msg )

That makes quite a bit of sense to me ... in fact, I would re-use that when implementing checkCachedCredentials.

The final alternative would be to let you supply an access token and a user and not check them with the backend -- just use them. However, that is essentially what will happen anyway if the backend can't be reached (for one reason or another) ... that is, we will end up in AuthenticatedUser state with the credentials cached anyway -- it is just that we'll fill in the relogin field to indicate the error. So, I think that we don't really lose anything by contacting the backend, even if (strictly speaking) it would be unnecessary in some cases.

amitaibu commented 6 years ago

So, I think that we don't really lose anything by contacting the backend, even if (strictly speaking) it would be unnecessary in some cases.

So to confirm, In my case I get the access token and the User immediately after they have registered - in that case we wouldn't be re-calling the backend, right?

rgrempel commented 6 years ago

So to confirm, In my case I get the access token and the User immediately after they have registered - in that case we wouldn't be re-calling the backend, right?

No, we would re-call the backend in that case -- that's the case where we call the backend when it wouldn't (strictly speaking) be necessary.

We basically wouldn't completely trust your supplied User. We'd take your access token and call /api/me to get the user from the backend, even though you supplied it. If that succeeds, we'll use the User the backend sends back from /api/me. If it fails, we'll still be in Authenticated state (with the User you supplied), but we'll indicate the error in the relogin field (so you know that we couldn't actually verify that the access token was good).

So, it's one call to /api/me that is, strictly speaking, unnecessary, since you're providing both the access token and the User. However, I think it's a good idea.

It allows us to know, from our own knowledge, that the access token will work with /api/me. This is handy, because we'll send it to /api/me the next time (when we get it from our cached credentials). So, this avoids a case where one mechanism works but the other does not -- that is, we only fully succeed if /api/me actually works with the access token.

If we want to avoid the extra call to the backend, we'd need to expose a message something like recordLogin ... I suppose it would look something like this:

{-| Record a successful login which you've performed independently.

This differs from `loggedIn` in that it will cache the credentials you supply.

It differs from `checkAccessToken` in that it won't contact the backend to
verify the credentials you supply.

-}
recordLogin : Credentials user -> Msg user

Actually, on second thought, this would be a very easy function to write now (after the changes so far in #16), so I may as well do it.

rgrempel commented 6 years ago

OK, now I've also got a recordLogin function on #16 which allows us to record and cache a successful login, without contacting the backend again.

rgrempel commented 6 years ago

This is now published, in version 2.1.0.