anitya icon indicating copy to clipboard operation
anitya copied to clipboard

Add authenticated JSON API to request upstream project monitoring

Open ncoghlan opened this issue 9 years ago • 21 comments

Currently the only way to explicitly request monitoring of a new project is to login with FAS and then use the /projects/new/ form submission endpoint at https://github.com/fedora-infra/anitya/blob/master/anitya/ui.py#L330

For programmatic use (building on #330 and as a pre-requisite for #318), it would be beneficial to be able to submit new projects for monitoring via an authenticated JSON API using the upstream ecosystem name and the name of the project within that ecosystem.

ncoghlan avatar Aug 08 '16 07:08 ncoghlan

I've started two relevant branches in my copy of the Anitya repo:

  • draft API for this and for #318 in https://github.com/ncoghlan/anitya/tree/upload-package-mapping
  • some initial experimentation with openid-connect in https://github.com/ncoghlan/anitya/tree/openid-connect (EDIT: this branch has been superseded by https://github.com/ncoghlan/anitya/tree/authenticated-api)

I'm thinking the appropriate structure for the upstream PRs would be:

  • initial PR that adds both OpenID Connect support and the ability to submit projects for monitoring (without the latter, the OIDC support seems a bit pointless)
  • second PR that adds the upstream/downstream mapping endpoint for #318

ncoghlan avatar Aug 18 '16 09:08 ncoghlan

I poked at this a little bit over the weekend. I'm pretty new to flask, so I'm a little torn on API libraries to use, but there seem to be several with promise. I experimented a little with restless. flask-restful looked interesting as well, as does flask-restless.

Do you have any particular opinions or preferences?

jeremycline avatar Dec 19 '16 21:12 jeremycline

Sorry for dropping the ball on this one (Hooray for shifting work priorities...).

For our current patches, we just used raw Flask+JSON, as we didn't want to get into the "Choosing a REST framework for upstream Anitya" discussion: https://github.com/bkabrda/anitya-docker/blob/master/patches/06-initial-API-for-mapping-uploads.patch

However, the project we're currently carrying those patches for uses flask-restful as the foundation for its REST API, and we haven't had any problems with that choice. So if that was considered an acceptable dependency to introduce, I'd happily rework my patches accordingly :)

ncoghlan avatar Dec 20 '16 04:12 ncoghlan

Great, I'm fine with adding flask-restful as a dependency. The argument parsing and automatic bad request help looks particularly nice and was something restless didn't help with much. Authentication looks easy to add, as well.

jeremycline avatar Dec 20 '16 15:12 jeremycline

I built on top of your draft a bit: https://github.com/jeremycline/anitya/tree/authenticated-api - it still needs tests and documentation. I've never worked with OpenID Connect, but theoretically https://github.com/fedora-infra/python-fedora/pull/188 implements the steps necessary to get a token. Since I'm about to go on vacation I thought I'd put it up here, but I might continue poking at it over the holiday.

jeremycline avatar Dec 22 '16 20:12 jeremycline

New branch in my repo based on @jeremycline's updates and getting the tests green again: https://github.com/ncoghlan/anitya/tree/authenticated-api

Figuring out a test strategy will be the next key step. Would it be OK to require folks running the tests to log in with oidc-register first so the client has valid FAS credentials and even the local server can just check against FAS as its identity store?

ncoghlan avatar Dec 26 '16 03:12 ncoghlan

I think that's reasonable to start with. If it becomes problematic we can re-visit it, of course.

jeremycline avatar Dec 26 '16 21:12 jeremycline

Big -1 to APP.config['OIDC_ID_TOKEN_COOKIE_SECURE'] = False. Please do NOT default to that.

puiterwijk avatar Jan 03 '17 15:01 puiterwijk

@puiterwijk I'm not sure what you're looking at, since it's not False. Somewhat related, it would be good if the settings in flask-oidc were documented. Right now there's no mention of them at all.

jeremycline avatar Jan 03 '17 15:01 jeremycline

@jeremycline sorry, seems I had been looking at an old commit. Regarding docs, you're completely correct. I was under the impression they were documented.

puiterwijk avatar Jan 03 '17 19:01 puiterwijk

I think that setting is false in my original just-for-initial-local-tinkering openid-connect branch, but has been fixed in the new branch based on Jeremy's work.

Regarding testing, I realised checking against FAS would only work for manual testing - in a CI environment, there's no human around to manually create a credentials file.

@puiterwijk How are you testing the OpenID authentication support in other Fedora Infrastructure apps?

ncoghlan avatar Jan 04 '17 05:01 ncoghlan

Latest update to https://github.com/ncoghlan/anitya/tree/authenticated-api splits the API tests into four categories as follows:

  • anonymous access (these are the existing API tests)
  • requires authentication (currently empty, will test that auth errors are received on authenticated APIs when not logged in)
  • mock authentication (CI friendly tests that will mock out the server's authentication checks to make them allow access)
  • live authentication (tests that will require a local client_secrets.json file for authentication against FAS and will be skipped otherwise)

The last two will share test definitions, they'll just have different setup steps.

ncoghlan avatar Jan 09 '17 03:01 ncoghlan

I'd missed that @jeremycline had moved the new Flask-RESTful based endpoint out to a new "v2" API definition. That's a good idea, and means the test cases can be put in a new test file that's specific to that version of the API.

ncoghlan avatar Jan 09 '17 04:01 ncoghlan

Latest commit on the branch refactors the API so that features that normally require authentication can be tested without needing an OIDC server in the test environment: https://github.com/ncoghlan/anitya/commit/6eae55db2d50f942a715d17481ac51169e0d0d48

It also ensures that the server can still be run without the authenticated API support configured at all

That means the last change needed should be updating the test client to actually send the OIDC access token when doing the live authentication testing (currently that fails with a 401 error, since the token isn't being sent).

ncoghlan avatar Jan 09 '17 07:01 ncoghlan

@puiterwijk Where can I find documentation on how to use client_secrets.json to request a valid OIDC access token from FAS?

ncoghlan avatar Jan 09 '17 08:01 ncoghlan

The most useful resource I've found so far is http://connect2id.com/learn/openid-connect#example-auth-code-flow, which I'm currently working through attempting to substitute in values from the client_secrets.json file, but it's really unclear how the theoretical concepts translate to actual working client code that can be used in a test suite.

ncoghlan avatar Jan 09 '17 08:01 ncoghlan

Looks like Digital Ocean have a good write-up of the underlying OAuth 2.0 mechanisms for the client authorization request and token update flows: https://www.digitalocean.com/community/tutorials/an-introduction-to-oauth-2

ncoghlan avatar Jan 10 '17 03:01 ncoghlan

It turns out that requests does support token retrieval, it just wasn't documented very well so I missed it when I first looked at the docs: https://github.com/kennethreitz/requests/pull/3804

So the latest version of the branch is actually pretty close to workable, with the optional integration tests confirming that it really can authenticate client tokens against FAS.

That means the open questions are now more design related:

  • the current patch doesn't offer any way to generate an access token that's valid for release-monitoring.org itself. Adding this will mean adding a way to initiate the token request flow from within the Anitya web application, and actually doing something useful in the "/oidc_callback" endpoint

  • the dynamic client registration used in the integration tests means we can't actually define segmented permissions for OIDC - it's all or nothing using the generic "oidc" scope. At the moment the patch defines the scaffolding for separate "upstream" and "downstream" scopes, but doesn't actually use them in practice. @puiterwijk, is there a way to automate registering new scopes with FAS for a dynamically registered client application? If not, it will likely make more sense to remove that scaffolding entirely, and be explicitly that the client permissions are all-or-nothing.

ncoghlan avatar Jan 10 '17 13:01 ncoghlan

Just for interest, the author of https://github.com/h2non/nightmare-google-oauth2 pointed out that you can actually take the token retrieval automation a step further and issue the authorization request from the command line application as well. Using that approach, we could prompt for the user's FAS password directly in the terminal rather than getting them to submit it via their browser. However, I don't think we need to go that far at this point.

ncoghlan avatar Jan 10 '17 13:01 ncoghlan

Talking to @puiterwijk at DevConf, he explained that the endpoint for reviewing (and revoking) granted permissions is at https://iddev.fedorainfracloud.org/portal

Reviewing that shows problems with the current dynamic client registration, as:

  • the client description is empty
  • the consent data is empty

That makes it difficult to identify ephemeral clients and revoke their access.

ncoghlan avatar Jan 27 '17 11:01 ncoghlan

Proper scopes are set up in the development FAS server now: https://fedoraproject.org/wiki/Infrastructure/Authentication#release-monitoring.org

That gives proper "consent data" entries for the dynamically registered test clients, and presumably the static client registrations for the live release-monitoring.org server will also have meaningful client descriptions.

ncoghlan avatar Feb 09 '17 18:02 ncoghlan