Apache Allura™

• labels: --> export

• Size: --> 2

Use API to trigger export, use API to poll for status & get filename (full connection path?).

Goal: have a script (or use a utility like curl-oauth to avoid the oauth complexities and be able to do it all in a shell script) to hit the new API to start an export. Then check the new export status API until the export is no longer busy, then the script can download the file.

• status: open --> in-progress
• assigned_to: Cory Johns

• status: in-progress --> code-review

allura:cj/6692
forge-classic:cj/6692
sfx:cj/6692

The forge-classic and sfx changes are for adding the OAuth account menu item, because the third-party apps section was removed from Subscriptions and integrated into the improved OAuth page.

• QA: Dave Brondsema

I like it. I have a lot of feedback though:

bearer tokens

• If I use an invalid access_token, I get a page with status 403 and but the body says "302 Found" and points to http://.../error/document
• minor: template var authorized_applications was removed in one place, it's also unnecessary in PreferencesController.index

Oauth page:

• Consumer Key is repeated for Consumer Secret
• the confirm dialog pops up for the wrong button

Being picky since security is important

• name=request_token.name, is added. Why?
• test_bearer_token would be better as 4 separate tests so we don't have to worry about changes to the mock & the request carrying over from one test case to the next.
• even more tests, like:
  • functional tests of the /auth/oauth/ page
  • functional tests where OAuthAccessToken queries are not mocked.
  • oauth tests (not your fault) but all we have is a few in allura/tests/functional/test_auth.py that only cover the /auth/oauth/ page (and 2 of those fail & need to be updated). We should write the tests since we're touching the functionality a little bit here.

backup APIs

• Shouldn't ProjectAdminRestController._check_security require 'admin' access? I can't think of a case where we'd want admin-level stuff to be exposed to those with just 'read' access.
• the export methods of ProjectAdminController and ProjectAdminRestController share a lot in common. What would you think of ProjectAdminController.export calling ProjectAdminRestController.export and converting any HTTP exception into a flash message? The API should have the bulk_export_enabled check too.
• splitting up test_export so each case has a name would help me at least (test_export_no_exportable_tools, test_export_i_don't_know_why_the_2nd_one_fails)

overall

Can we provide a sample shell script that uses the API? We also need to document the API. Particularly note the http codes, e.g. so people know 503 means task busy not server having problems.

• status: code-review --> in-progress

• Milestone: forge-oct-04 --> forge-oct-18

ProjectAdminRestController._check_security checks for read so that the export status check doesn't have to be authenticated. But, I guess if the script that queued the export is already authenticated, re-using the auth for the status check isn't a big deal.

I noticed the body of the 403 page but I have no idea how that's happening, as I'm just raising a standard HTTPForbidden exception. Maybe I should return None like the other parts of that method do, but that just allows anon access with no error indicating that the auth failed (unless anon is disallowed later on).

The name=request_token.name was a part of a rolled-back change, and should have been removed.

Yeah, I think it's safer to require 'admin' for that whole controller

403: Right, certainly unrelated code causing that. I guess see if its a quick fix? If not, ticket up with what you did find out.

• status: in-progress --> code-review

Changes pushed to allura:cj/6692

• summary: Exports should be scriptable --> Exports should be scriptable; Simple oauth API via bearer tokens

• status: code-review --> in-progress

[d6d1e6] removed the revoke_oauth method and its functional test. But it's still referenced by OAuthRevocationForm

What about these items from my previous comments?

• break apart the test_bearer_token test into separate tests
• "even more tests" ...
• sample shell script using API
• export API documentation

• Milestone: forge-oct-18 --> forge-nov-01

I forgot to mention: with the change to _check_security, the 302/403 issue resolved itself.

• status: in-progress --> code-review

allura:cj/6692 (rebased and force-pushed)

Oh, I was wrong, but I was able to fix it.

Not exactly sure what you mean by "even more tests." Allura/allura/tests/functional/test_auth.py:TestOAuth already has functional tests of all the functionality on the /auth/oauth page, and I converted the valid case of the bearer token tests to a functional (non-mocked) test. Are you suggested we add tests to cover the interactive oauth process?