Restructure MembershipService and Authprovider

[RedX2501]

As noted by Lars in #482 (comment) there is clearly to much going on in the MembershipServices.

IMHO we should remove all Authentication related code from the MembershipServices and force AD users to activate WindowsAuth.

That way {AD,EF}MembershipServices do no Authentication (password checking) but only provide User/Role information.

AD will then require windows auth.

EF will be able to choose between windows auth and cookie.

[willdean]

You're clearly right here. The fact that ADMembershipService only implements about two methods of its interface is a pretty sure sign that it's the wrong thing in the wrong place.

If we're going to re-architect this stuff, is there any reason why AD couldn't use the EF back-end for storage?

As far as I can tell, all that ADBackEnd actually does is make sure that the ADBackEnd database is kept in sync with the the AD Server.

At the moment ADRoleProvider and ADTeamRepository and ADRepositoryRepository, ADRepositoryPermissionService do nothing that isn't done by their EF equivalents, they just use a different storage system (and because they're a parallel implementation they have various bugs and subtle differences)

If we say there are only two operations to Authentication:

• Look up the string username in a Users database, and make sure we know who they are
• Verify their password

The the second part is perhaps the only thing which needs to be handled in an 'AD-specific' way, though perhaps if we don't find a user in our local database we need to go to the AD server to see if they're a new arrival who we don't know about yet.

The first part could be done in one place, so that every possible perversion of searching for user/domain, user@domain, user, USER, user-grandmother@address.yahoo, user-cat's-maidenName.givenname could all be carried out in one function, not vomited all over the whole application.

AD will then require windows auth.
EF will be able to choose between windows auth and cookie.

I'm not sure if this is really 100% essential, but thinking that it's not might be the same mistake that's already been made! At least adding some restrictions here would reduce the size of the test/support space.

[RedX2501]

AD requires windows auth because we never see the plain text user passwords.

EF with windows auth is nice because this allows me to use the existing infrastructure of usernames and passwords provided by my company while not having to bother admins about git groups. This is also nice for the users as they dont have to create and manage another login within the company.

EF with cookie is nice because allows the user to stay logged in to the system.

IMHO the new design should be something along:

Cookie login is always enabled for all authentication providers as that allows you to login from your browser automatically if you logged in before. This should not be an option and be provided by default when you select an auth provider.

Auth providers:

• Internal: Uses the information from the local db. This means username and passwords.
• WindowsAuth: Uses the windows authentication mechanism. This means we never see a password.

Membership providers:

• Internal: Uses the information from the DB to provide correlation between repos/groups and users.
• AD: Uses ActiveDirectory to provide correlation between the user and groups. Still uses the internal DB to provide correlation between the Users/groups and repositories.

If someone can tell me what Federation does that would be nice. Then we can fit it in into the above "table".

[larshg]

AD requires windows auth because we never see the plain text user passwords.

Isn't it the other way around? Windows authentication requires AD membership?
When windows authentication is used - what do we get for identifying the user? And where in bonobo is the first executing happening?

If you use cookie authentication we get navigated to Logon in HomeController?

if (ModelState.IsValid)
            {
                ValidationResult result = MembershipService.ValidateUser(model.Username, model.Password);
                switch (result)
                {
                    case ValidationResult.Success:
                        AuthenticationProvider.SignIn(model.Username, Url.IsLocalUrl(model.ReturnUrl) ? model.ReturnUrl : Url.Action("Index", "Home"));
                        Response.AppendToLog("SUCCESS");
                        return new EmptyResult();
                    case ValidationResult.NotAuthorized:
                        return new RedirectResult("~/Home/Unauthorized");
                    default:
                        ModelState.AddModelError("", Resources.Home_LogOn_UsernamePasswordIncorrect);
                        Response.AppendToLog("FAILURE");
                        break;
                }                
            }

First it uses the MembershipService to validate the users username and password? (which in current state can be both AD and internal?

Then it uses the AuthenticationProvider to "sign in" - is this what creates a cookie for further "request" ?

So the AuthenticationProvider doesn't really authenticate the username and password?

I think I need to find some articles about this - I thought I almost had it and could do some proper PRs about this - but the more I get into it, the more I find out that I don't know :P

Edit:

Found this at stackoverflow - bonobo doesn't create a token/cookie - and saves it in the db? Or what is saved in the "cookie"?

[larshg]

I just tried to debug after logging in with cookie authentication. I can see subsequent access to various pages is handled in WebAutherizeAttribute - where the httpcontext has the user which was created during cookie authentication?

So if you use windows authentication and it uses the Kerberos or NTLM or any other, this would be where the first execution would be - and here the user should already be populated with? data from AD?

[willdean]

@larshg The cookie has a the claims in it, encrypted.

[stupiduglyfool]

@willdean The federation provider allows passive single sign on with an ADFS relying party. During login you are forwarded to the adfs page to provide credentials, if the browser sends a valid kerberos token its possible that you won't be prompted for credentials. Then adfs provides a SAML token back which includes the identity of the logged in user and the issued claims.

If you don't mind me suggesting, I think you should look at something like incorporating IdentityServer3, then you wouldn't have to worry about user or account management at all..

[willdean]

@stupiduglyfool I've only been involved in the Bonobo code for about a week, so you may suggest anything and everything without offending me! Dominick Baier is pretty much god-of-auth as far as I'm concerned, so it wouldn't take much to sell me on using IS3, if it could be incorporated without making a deployment nightmare.

Auth in MVC apps doesn't need to be as hard as it seems to be in Bonobo, which I think is just a product of the evolution.

Have you used IS3 yourself?

[RedX2501]

So this means that we would use federation for auth but still need the db for user/role matching?

This means
Internal db with windows auth, federation and internal.
Ad with windows auth.

I'm not sure if it's worth the effort of integrating IS3 considering that the Auth providers are already implemented. Maybe not in the most optimal way but they are there.

[larshg]

Any reason why you can't use cookie auth with AD backend?

[RedX2501]

It could but then the adbackend must provide code to query the userprincipal for password, which i think does not belong there.

[larshg]

Yes, thats true.

[willdean]

@RedX2501 I think the suggestion would be to use IS3 to do the complete user database (it has an EF-based storage option). But it is a huge change, and it seems to me too that we're not really that far from where we need to be with a certain amount of tidying / reorganisation.

[stupiduglyfool]

@willdean Yes, we use IS3, I agree it would be a huge shift in your code, but since this is discussion of an overhaul of membership and authentication, I just thought it was worth bringing to the conversation for consideration, as it would allow you to focus your project on what it does best. I think there would be other potential benefits such as authorization, protecting branches with rights..

Anyhow, I hope you don't mind me contributing to this discussion.. I am a relative new comer to both git and bonobo, having just started using it in November. I'm successfully using ADBackend with Federation (ADFS). The benefits of this are that we already have users and groups set up in active directory, the ldap synchronization provided saves us from repeating that.. Therefore I would like to suggest that during any refactor this isn't lost as it is very convenient.

[willdean]

@stupiduglyfool A lifetime of working in consultancy has clearly allowed me to convince you that I'm much more important to the Bonobo project than I really am - I've been using it for a couple of years but working on it for a couple of weeks. Its @RedX2501 who is the senior one here.

I would hope everyone else is really grateful for your contributions, particularly on the federated stuff.

I don't fully comprehend the discussion above between @RedX2501 and @larshg about the relationship between using AD/LDAP to populate the list of members and whether we have cookies or windows auth between the browser and the server.

It's possibly a bit unfortunate that the AD implementation has separated/duplicated more code than was necessary - it if just held the EF database in sync with the LDAP server and then, separately used the AD server for authentication where necessary, then it would all be a lot easier to reason about, partly because there would only be two blobs of AD code - the Database<-LDAP synchroniser and the AD authentication code.

[RedX2501]

@stupiduglyfool I'm not expert in Federation Auth so let me try to understand your setup.

You have an LDAP server setup. Windows Auth cannot be used in this context to authenticate the users. AD cannot be used to query LDAP servers, so you need ADFS to convert your LDAP setup to a format that is understood by AD. Is that correct?

And to confirm what willdean said, i too am very happy to have someone helping us that can test and contribute ideas to the federation side. I have never used IS3 so I don't know what it would replace in the code. From the short tutorial i read it seems it would need to run another application? If that's the case i'm definitely against incorporating it.

[stupiduglyfool]

@RedX2501 Sorry to confuse you, we use AD, I assumed that the synchronization bonobo used an LDAP connection to provide the teams and users synchronization.

We use ADFS to provide single sign on capabilities, although windows auth could be used internally, adfs is better suited for external access.

IS3 can be ran together in one application, using owin/katana, you can have one application self hosting multiple endpoints.

[willdean]

@RedX2501 The ADFS server is just doing Authentication. @stupiduglyfool is using ADFS to do authentication, but using AD to populate the membership database. Under the covers AD is using LDAP to do this, but there's no explicit code in Bonobo which talks about LDAP, because it's hiding under the AD wrappers.

[willdean]

I'm with @larshg that we shouldn't stop supporting form+cookie authentication for people using the AD membership.

I think the matrix looks like this:

Form+Cookie Auth: Internal membership or AD membership
Windows Auth: AD membership only
ADFS Federation Auth: AD membership only (This will also be using a cookie, but it's a different one)

By "membership" I'm meaning "how the list of users, teams & roles gets provided".

[RedX2501]

My use case as described above is Windows Auth + Internal DB which you do not have in your table. #483 (comment) 2nd paragraph

AD membership with Form + cookie makes no sense because at first login you need to auth. If you do that with forms you must have a password to check against in the DB which you don't. AD does not provide you with this information.

If #489 goes though we will only have one database, then it is irrelevant If there is EF or AD. The ONLY membership provider will do User/Role/Repository matchi

The settings we now have called MembershipService will only tell you from where you fill your database. I propose we rename it to DataProvider

1. Forms: every time a user logs in (doesn't matter which of the 3 Authentication provider is used) a user is added to the database
2. AD: A background job takes care of filling the database with data from the active directory service.

Then we are left with authentication. Here i suggest the followin names:

1. Forms: Not to be confused with the data provider above! Requires to have the Password in the database. This cannot happen when the data is coming from AD as you don't have the plain password to encrypt. This is your typical form auth + cookie.
2. Winodws Authentication: This uses the windows domain to see if the username and password provided match. It will set a cookie to avoid having to type in the username and password again for a certain time. Since the password stored in the database is irrelevant for this
3. Federation Authentication: This uses the Federation authentication provider to see if the username and password match. It will also set a cookie to allow for a easy re-login.

So all 3 have cookie.

the table becomes

         | Forms | AD
---------+-------+-----
Forms    |  X    |
Windows  |  X    | X
Fed      |  X    | X

Is it more understandable now?

[willdean]

You can currently, in the current code, use form+cookie auth to log in to an AD membership system. The ADMembershipService does this by asking the AD server to validate the password.

[willdean]

I wonder if the confusion here is because we're talking about three concepts with two terms. The three considerations are:

1. How we populate the database which we always need for authorization decisions and sometimes need as a password hash store.
2. How the Bonobo server app decides if someone is authenticated
3. How the client sends credentials to Bonobo

So, 1 is either internal (enter them on web-pages) or import them from AD.

and 2 is by checking name+password against internal database, or by checking name+password against AD, or by taking an ADFS token, or some NTLM native magic which the WindowsAuth code uses.

and 3. Is by 'HTTP Basic auth' for the Git clients, or by presenting a cookie which bonobo generates, or by presenting a cookie which ADFS generates, or by Windows auth

Does that make any sense?

[RedX2501]

I know about cookie + ad, and as i tried to explain above i think this is a misconstruct which should not exist.

Is it really so important to not get the pop-up but instead have a form on the page?

[RedX2501]

@willdean Are you trying to understand how it current works or how it should be?

In post #483 (comment) I'm trying to explain how i see it after the refactoring. Independently of what we have now. I'm trying to find new names so we can keep some distinction.

Is the distinction in my post unclear? Or have it made it too hard to understand the relationship between the actions and the components?

[willdean]

Ahhh. I have misunderstood you as saying 'we can't offer form login with AD membership' because it's hard/impossible in some way, whereas you're actually saying we do have it but should drop it because it's unnecessary.

I guess that's not for me to say, really - it's a policy decision about the project which I don't feel able to make. My position was only 'here's all the functionality we have, how do we provide that cleanly', yours is more ambitious as 'what subset of functionality could we retain which would lead to a cleaner implementation'. I'm not going to interfere in that.

My parting shots will be:

• You're always going to need to be able to do name+password auth against AD to support git clients which do basic-auth, so even if you drop the ability to log-in from a web form then you haven't avoided that.
• I think using the word 'forms' twice to describe two orthogonal concepts is potentially confusing.

[RedX2501]

The git part is problematic. I guess currently it is only working out of sheer luck because people doing Federation Auth are using ADMembership, or Windows Auth. If they changed to Internal they won't be able to login. @stupiduglyfool can you verify this?

[willdean]

I don't think anyone could do federation with internal membership, because then they'd need to manually copy all their users out of the AD database which ADFS is using into the internal database.

[RedX2501]

It's the same as windows auth with internal. On first login the user is then added to the internal DB.