662859 - Adding node checking

Status: RESOLVED FIXED

(Cloud Services Graveyard :: Server: Sync, defect)

Description

[Tarek Ziadé (:tarek)]

The storage server needs to control the user node on auth

Comment 1

[Tarek Ziadé (:tarek)]

Attachment: authenticate_user can return fields

This is the first patch required: authenticate_user can now receive a "fields" option, that contains field names that will be extracted from the user data in the DB.

For instance we can do:

  authenticate_user(name, password, fields=['userNode'])

The return type of the API changes consequently: it's now a tuple of value. The first one is the user id, followed by the values of the claimed fields:

  >>> authenticate_user(name, password)
  (123, )

  >>> authenticate_user(name, password, fields=['userNode'])
  (123, ['weave:balbalba',])

If the user is not recognized or the auth fails, it returns None as usual.

The server-storage app will hae the ability to check the node value right after the authentication happens.
 
This change impacts all applications that use this API, which is fine as long as we deal correctly with the dependencies versions in them.

e.g. if an app is not changed, if can do: "server-core <= 1.6" , otherwise : "server-core > 1.6"

Comment 2

[Toby Elliott [:telliott]]

Comment on attachment 538225 [details] [diff] [review]
authenticate_user can return fields

This is fine, but note that the semantics are going to change again with the user rewrite. It's going to look like this:

user = User()
user['userName'] = 'foo'
uid = auth.authenticate(user, password, ['syncNode'])

Assuming a successful call, user['userId'] and user['syncNode'] will be populated

Comment 3

[Tarek Ziadé (:tarek)]

Well maybe we should wait then because we'll be back at the current return type (id or None).

Do you think your auth work will be over within 2 weeks ? If so, let's hold this refactoring so we're not bothered by changing the return type back and forth in all applications.  If so please just mark this bug as blocked by your meta-bug on auth changes

Comment 4

[Toby Elliott [:telliott]]

I think the code will be done in a couple weeks, but there's a lot of migration work too (see the ldap change bug). Should have the changes to account-portal out today for a working example.

Comment 5

[Tarek Ziadé (:tarek)]

Attachment: temporary patch to reject users on the bad node

Comment 6

[Tarek Ziadé (:tarek)]

Attachment: temporary patch to reject users on the bad node (revised)

Comment 7

[Toby Elliott [:telliott]]

Comment on attachment 541419 [details] [diff] [review]
temporary patch to reject users on the bad node

r+ with the addition of a host in the last chunk.

Comment 8

[Tarek Ziadé (:tarek)]

done in https://hg.mozilla.org/services/server-core/rev/0a1f393f83ae

Comment 9

[James Bonacci [:jbonacci]]

Attachment: QA Functional verification