colinskow / superlogin

Powerful authentication for APIs and single page apps using the CouchDB ecosystem which supports a variety of providers.
MIT License
370 stars 116 forks source link

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

Open delkant opened 8 years ago

delkant commented 8 years ago

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.

micky2be commented 8 years ago

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.

colinskow commented 8 years ago

@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.

delkant commented 8 years ago

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.