nicolasff / webdis

A Redis HTTP interface with JSON output
https://webd.is
BSD 2-Clause "Simplified" License
2.82k stars 307 forks source link

Basic auth doesn't work #136

Open vyarovoy opened 7 years ago

vyarovoy commented 7 years ago

Hi,

Even with ACL section defined as following "acl": [ { "http_basic_auth": "user:password1", "enabled": ["SET", "DEL"] } ]

I'm still able to GET values with Authorization header compiled from wrong values (user:password , without trailing 1)

Hence I suggest Basic auth feature doesn't work properly

cvanarsdall commented 5 years ago

I'm struggling a lot with AUTH myself. The documentation doesn't seem to be enough for me, it also doesn't line up with my understanding of redis.

For example, is the AUTH a webdis side or is it a passthrough to Redis? Redis doesn't seem to support a user, just a password, so why does webdis have a user? Is there a way to get the logs to print out the AUTH communication with Redis? Is there a verbosity level that tells you different reasons why webdis rejects requests? For example, will it ever tell me I didn't add a particular commanad to an acl?

I'd love some help in diagnosing what might be wrong as this AUTH stuff is very important to my sysadmin.

cvanarsdall commented 5 years ago

Okay, I found more info. 1.) The webdis.json is very sensitive to the data type put into the json. I put a true (boolean) but the config parser expects a string here.

2.) Now this is what I'm currently stuck on. I'm in the acl code and it's verifiably not sending any passwords anywhere. What appears to be happening is that there's a header grab for 'Authorization' and that's returning null. It's this code from acl.c which in this example is line 18:

13 int 14 acl_match_client(struct acl a, struct http_client client, in_addr_t ip) { 15 16 »·······/ check HTTP Basic Auth / 17 »·······const char auth; 18 »·······auth = client_get_header(client, "Authorization"); 19 »·······if(a->http_basic_auth) { 20 »·······»·······printf("Basic auth sanity\n"); 21 »·······»·······printf("Auth is: %s\n", auth); 22 »·······»·······if(auth && strncasecmp(auth, "Basic ", 6) == 0) { / sent auth / 23 »·······»·······»·······if(strcmp(auth + 6, a->http_basic_auth) != 0) { / bad password / 24 »·······»·······»·······»·······printf("webdis thinks it's a bad password\n"); 25 »·······»·······»·······»·······return 0;

Also - someone who actually knows C might correct me (i'm a python developer) - but is that string comp skipping the first five characters, that is the 'user:' in the http_basic_auth example?

cvanarsdall commented 5 years ago

ah - okay I understand the null value from the header grab - that's when the browser doesn't supply auth credentials. If someone else doesn't get to it before me, a lot of additional logging here would be super helpful in the future.

nicolasff commented 5 years ago

Thanks for looking into this. Unfortunately I’m unable to work on webdis at this time but I’d be happy to review a patch and tag a release as I’ve done for other issues. My apologies, this is mostly out my control right now.