Closed Bug 865936 Opened 11 years ago Closed 11 years ago
server-whoami; a simple API for getting your sync account details
This is a tiny little server with a single API endpoint. You "GET /whoami" and it returns a JSON mapping with your sync userid and node assignment. If you don't provide credentials, or provide bad ones, then you get a 401. We intend to use this as part of a caching proxy auth setup, when deploying storage nodes into AWS, as described in Bug 865107. This is the component that would live inside our infrastructure and provide information to be sucked in to AWS. Doing a separate server means we can keep it more efficient and more secure. Attaching the entire initial state of the repo as a zipfile for review. I didn't use cornice for this, since the API is so tiny and we need so much of the stuff from server-core anyway, I didn't see the point in diverging any further from the way existing sync apps are build and deployed. One current annoyance is that it makes two calls to LDAP - one to authenticate the incoming request, and another to read the syncNode for the authenticated user. This *can* be done in a single LDAP request, but it means duplicating some of the authentication logic from server-core. I'll look into just how much work that would be, but wanted to get the rest of this out there ASAP.
Attachment #742118 - Flags: review?(telliott)
And here's a quick way to get this down to a single LDAP call - monkey-patch the authentication class to always load the 'syncNode' property, even if explicit node-checking is disabled. I'm not pleased about monkey-patching, but it avoids having to fiddle with the existing auth flow which includes sanity checks, cef logging, etc. Too nasty?
Attachment #742123 - Flags: review?(telliott)
Comment on attachment 742123 [details] [diff] [review] monkey-patch auth to always load syncNode property The monkey-patch is OK, but I think we might be better served by adding an autoload_node config value that just does this directly in the core.
Attachment #742123 - Flags: review?(telliott) → review+
Comment on attachment 742118 [details]
server-whoami initial code
Not a lot to talk about here. It's all pretty straightforward.
init check for old-style auth backends seems a little unnecessary at this point. It's been like 2 years since everyone swapped over, and, given the nature of this piece, nobody is likely to install this with a non-compatible backend. From an efficiency standpoint it's pretty irrelevant, but it does represent extra code that could go wrong or get mangled in the future.
get_user_details comments are out of date thanks to the monkeypatch
Attachment #742118 - Flags: review?(telliott) → review+
(In reply to Toby Elliott [:telliott] from comment #3) > init check for old-style auth backends seems a little unnecessary at this > point. It's been like 2 years since everyone swapped over, and, given the > nature of this piece, nobody is likely to install this with a non-compatible > backend. From an efficiency standpoint it's pretty irrelevant, but it does > represent extra code that could go wrong or get mangled in the future. I still find references to old-style classes from time to time, e.g. in some of the .ini files we use for testing. Unless you feel strongly otherwise, I'd like to leave it in just to guard against wasting time tracking down such a mis-match. Committed in: https://github.com/mozilla-services/server-whoami/commit/ebf227b0c63e76a391e45cda4d1f1bf49a2e7b78
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Created Bug 866562 for eventually replacing the monkey-patch with something more elegant. Not worth delaying the stage deployment of the code to implement that.
These changes will be verified as part of the following deployments: Bug 866568 - Deploy server-whoami into staging environment Bug 866573 - Deploy trial server-storage nodes in AWS; server-storage -> 1.15-8, server_core -> 2.13-1
Verified code installed from https://github.com/mozilla-services/server-whoami repo (see Comment 4).
Hmmm, formally Verifying this...
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.