#6692 Exports should be scriptable; Simple oauth API via bearer tokens

v1.1.0
closed
General
2015-08-20
2013-09-20
No

Currently difficulties in project exports are:

  • requires a manual form submission
  • email is used to notify when ready
  • resulting filename is non-deterministic

It should be possible to automate a project export with a script, with minimal difficulty.

Related

Tickets: #4633

Discussion

1 2 > >> (Page 1 of 2)
  • Dave Brondsema

    Dave Brondsema - 2013-09-20
    • labels: --> export
     
  • Dave Brondsema

    Dave Brondsema - 2013-09-20
    • Size: --> 2
     
  • Dave Brondsema

    Dave Brondsema - 2013-09-20

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

     
  • Dave Brondsema

    Dave Brondsema - 2013-09-23

    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.

     
  • Cory Johns

    Cory Johns - 2013-09-24
    • status: open --> in-progress
    • assigned_to: Cory Johns
     
    • Libri Nozze

      Libri Nozze - 2016-07-28
       

      Last edit: Libri Nozze 2016-08-05
  • Cory Johns

    Cory Johns - 2013-09-26
    • status: in-progress --> code-review
     
  • Cory Johns

    Cory Johns - 2013-09-26

    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.

     
  • Dave Brondsema

    Dave Brondsema - 2013-10-04
    • QA: Dave Brondsema
     
  • Dave Brondsema

    Dave Brondsema - 2013-10-04

    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.

     
  • Dave Brondsema

    Dave Brondsema - 2013-10-04
    • status: code-review --> in-progress
     
  • Dave Brondsema

    Dave Brondsema - 2013-10-04
    • Milestone: forge-oct-04 --> forge-oct-18
     
  • Cory Johns

    Cory Johns - 2013-10-04

    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.

     
  • Dave Brondsema

    Dave Brondsema - 2013-10-04

    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.

     
  • Cory Johns

    Cory Johns - 2013-10-07
    • status: in-progress --> code-review
     
  • Cory Johns

    Cory Johns - 2013-10-07

    Changes pushed to allura:cj/6692

     
  • Dave Brondsema

    Dave Brondsema - 2013-10-15
    • summary: Exports should be scriptable --> Exports should be scriptable; Simple oauth API via bearer tokens
     
  • Dave Brondsema

    Dave Brondsema - 2013-10-15
    • status: code-review --> in-progress
     
  • Dave Brondsema

    Dave Brondsema - 2013-10-15

    [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
     
  • Dave Brondsema

    Dave Brondsema - 2013-10-21
    • Milestone: forge-oct-18 --> forge-nov-01
     
  • Cory Johns

    Cory Johns - 2013-10-22

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

     
  • Cory Johns

    Cory Johns - 2013-10-22
    • status: in-progress --> code-review
     
  • Cory Johns

    Cory Johns - 2013-10-22

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

     
  • Cory Johns

    Cory Johns - 2013-10-22

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

     
  • Cory Johns

    Cory Johns - 2013-10-22

    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?

     
1 2 > >> (Page 1 of 2)

Log in to post a comment.