search

dennisreimann / masquerade

masquerade is the predecessor of masq: https://github.com/dennisreimann/masq - please consider using masq from now on, as it is the more modular approach.
MIT License
218 stars 46 forks source link

Fixed some bugs and added additional config options #26

Closed nougad closed 13 years ago

nougad commented 13 years ago

Hi,

I fixed some bugs in masquerade:

Additionally I want no auth in masquerade. My apache2 server implements a single sign login for all sites so I made some commits to support this:

Important is the commit to trust the basic auth and don't check the password in masquerade database. Also a account can be created on demand if create_auth_ondemand is set.

Finally I hide the logout link if user is logged in via auth_basic:

One last problem exists: I don't know how to enable read access to account page (and xrds). Currently I forbid all access and whitelist the required links:

< Location "/" >
  AuthName "login"
  AuthType Basic
  require group openid
  ...
< /Location >

< LocationMatch "^/(consumer|server|stylesheets|images|javascripts|help|logout|404.html|422.html|500.html|favicon.ico|robots.txt|safe-login)" >
  Order Allow,Deny
  Allow from all
  Satisfy Any
< /LocationMatch >

< LocationMatch "^/$" >
  Order Allow,Deny
  Allow from all
  Satisfy Any
< /LocationMatch >

But I need to enable /username(.xrds) as well for GET requests. But I don't want to write all usernames down. Do you have any ideas? My suggestion is to move /username(.xrds) to /users/username(.xrds) than a whitelist for /users/* would be possible. What do you think?

Greetings, nougad

dennisreimann commented 13 years ago

Thanks a lot for your contribution, this looks good! Unfortunately I cannot pull this in unless you provide tests for the scenarios you've implemented. These are some important changes to how authentication works and I'd be glad to accept them, but we'll have to require some verification that this is working as expected. Thank you!

nougad commented 13 years ago

Yeah, of course your right. I was so stupid to not run the tests (neither writing some). I found some errors in my code but I found some errors in the main project to while I fixed the tests:

I try to write tests for my features in the next days.

nougad commented 13 years ago

OK, finally the unittests should cover everything. I tried to make some diagram (graphviz) for authentication. (see commit 40bf5cd0593622904e945e7da2dfe8359ba25b77). Perhaps this chart helps to understand. Please review the code. Any comments are welcome.

Only my problem with url's are still exist. What do you think about to move /username(.xrds) to /users/username(.xrds)?

I forgot to mention the output

"warning: peer certificate won't be verified in this SSL session"

when I test d5b335f8b3b96c19b15218aa67d27daf30022981 - Any Ideas?

dennisreimann commented 13 years ago

Thank you, Florian. I've applyed your changes to the integration branch and will review them. Can you help me with that, @djmaze ?

dennisreimann commented 13 years ago

For me the tests are working without the mentioned problem, but I'll investigate what's wrong with that.