superlogin icon indicating copy to clipboard operation
superlogin copied to clipboard

Basic instead of Bearer? "Authorization": "Basic {token}:{password}"

Open delkant opened this issue 8 years ago • 3 comments

Hi there!

Let me start with: Very, very nice Project!! Because it really is.

Why you are using "Bearer" for the login when it is really a "Basic" authorization?.

I am asking this because when you are using libraries like okhttp3 that can handle this type of authorization you will need to do custom workarounds in order to make it work with superlogin. Please take a look here: https://github.com/square/okhttp/blob/master/okhttp/src/main/java/okhttp3/Credentials.java#L32

I am not asking you to change this to "Basic" but maybe allow us to use this String indistinctly if you think this is right, I am saying this because maybe you have a good reason to use "Bearer" in the Authorization header in your implementation and I just don't know what that reason is.

Thank you.

delkant avatar Aug 30 '16 16:08 delkant

It uses Bearer because it's using authentication through a token shared between the server and the client. You will find similar scheme with OAuth2.

micky2be avatar Aug 31 '16 05:08 micky2be

@delkant I originally used basic authorization and if the credentials are incorrect the browser automatically displays a username/password box. There is no way to disable this behavior, but it does not happen with bearer.

Most decent HTTP libraries will let you set custom headers. Check the source code of NG-Superlogin or Superlogin-Client to see how to do it.

colinskow avatar Aug 31 '16 10:08 colinskow

I see, thank you guys for the detail. I am working on Android with couchbase lite that uses this HTTP library internally (okhttp3).

In order to make my Android App to work with superlogin, I had to modify the util script where you extract the token and I just added a "or if basic" to util.getSessionToken.

https://github.com/colinskow/superlogin/blob/master/lib/util.js#L102

if (/^Bearer$/i.test(scheme) || /^Basic$/i.test(scheme))

This is a temporary workaround, I will change it to have just one regex to allow both Strings.

delkant avatar Aug 31 '16 13:08 delkant