#1687 Remove pre-oauth stuff

v1.2.0
closed
nobody
General
2015-08-20
2011-03-09
No

Remove the user preference for the key & secret key, and integration therein. Remove RestClient, remove server-side validation, etc.

Related

Tickets: #1687

Discussion

  • Dave Brondsema

    Dave Brondsema - 2011-03-29
    • milestone: backlog --> apr-7
     
  • Dave Brondsema

    Dave Brondsema - 2011-04-27
    • labels: --> cleanup
    • milestone: backlog --> may-05
     
  • Dave Brondsema

    Dave Brondsema - 2011-04-28
    • size: --> 2
     
  • Mark Ramm

    Mark Ramm - 2011-05-19
    • milestone: may-26 --> jun-02
     
  • Kyle Adams

    Kyle Adams - 2012-08-07
    • status: open --> in-progress
    • assigned_to: Kyle Adams ♞
    • milestone: someday --> forge-aug-10
     
  • Dave Brondsema

    Dave Brondsema - 2012-08-09
    • milestone: forge-aug-10 --> dir-aug-10
     
  • Kyle Adams

    Kyle Adams - 2012-08-22
    • status: in-progress --> open
    • assigned_to: Kyle Adams ♞ --> nobody

    • milestone: dir-aug-24 --> forge-backlog

     
  • Dave Brondsema

    Dave Brondsema - 2014-04-08
    • labels: cleanup --> cleanup, 42cc
    • status: open --> in-progress
     
  • Igor Bondarenko

    Igor Bondarenko - 2014-04-28

    I've deleted key & secret from preferences page, and also controllers for managing those.

    AFAIU, you want to get rid of authentiction by ApiToken (and friends) and keep only auth through OAuthAccessToken.

    In that case the following must be done:

    • Delete ApiToken & ApiTicket models and all related stuff
    • Change REST API tests to use authentication through OAuthAccessToken
    • Delete RestClient
    • Delete scripts/tracker-rip.py? It depends on RestClient. I assume that this is some old script that isn't needed anymore. Or maybe keep it, but change auth to oauth?
    • Delete integration in forge-classic (used only in SFXUserApi to set api_key for user, AFAICS)

    Just want to make sure we're on the same page here.

     
  • Dave Brondsema

    Dave Brondsema - 2014-04-29

    Yes to all of that. However, one concern is that AlluraImportApiClient is based on the old token/ticket model. The ticket import functionality (documented at docs/migration.rst) is probably something that could be removed since the newer ForgeImporter framework supports uploading ticket data in allura format. But the AlluraImportApiClient class is referenced in a few places and will have to be removed - I took a quick look at it seems like probably not too hard. I was afraid that the ForgeImporter classes were calling it somehow, but it seems not.

     
  • Igor Bondarenko

    Igor Bondarenko - 2014-05-02

    Ok, tracker import can be removed, but there are also wiki and forum import functionality in scripts/allura_import.py, which use AlluraImportApiClient. They should use oauth instead of old token model?

     
  • Dave Brondsema

    Dave Brondsema - 2014-05-02

    I didn't even realize that forums had that import functionality :) Well, I kind doubt the wiki & forum import parts of that script are used by anyone, but if they still work it'd be better not to remove them. I guess you could explore using an oauth model for bulk import and see if that's practical.

     
  • Igor Bondarenko

    Igor Bondarenko - 2014-05-06

    Ok, I've created another ticket for that on our side, since I've underestimated how much must be changed originally.

     
  • Igor Bondarenko

    Igor Bondarenko - 2014-05-06
    • status: in-progress --> code-review
     
  • Igor Bondarenko

    Igor Bondarenko - 2014-05-06

    Closed #574, #582.

    Changes:
    - allura:je/42cc_1687
    - forge-classic:je/42cc_1687
    - tracwikiimporter:je/42cc_1687

    Some notes:

    • To test any import script go to /auth/oauth/, generate bearer token and use it as -t parameter
    • I have changed forum import to use oauth, but wasn't able to test it. I just didn't find data in proper format to feed that script (output format of /p/test/admin/export is not what script expects, and also I didn't find any other discussion export capabilities)
    • I've also changed wiki import. This I was able to test (through trac importer and as a standalone script as well).
    • docs/migration.rst should be changed to indicate that allura_import script uses oauth bearer tokens now (and also tracker import does not exists anymore)
     
  • Dave Brondsema

    Dave Brondsema - 2014-05-12
    • status: code-review --> in-progress
    • QA: Dave Brondsema
     
  • Dave Brondsema

    Dave Brondsema - 2014-05-12

    Looking good. I did get an error when running the trac wiki import from the web interface (either from /p/import_project/trac/ or under admin, export menu).

    Traceback (most recent call last):
      File "/home/dbrondsema/dbrondsema-1019/forge/Allura/allura/model/monq_model.py", line 267, in __call__
        self.result = func(*self.args, **self.kwargs)
      File "/home/dbrondsema/dbrondsema-1019/forge/ForgeImporters/forgeimporters/base.py", line 131, in import_tool
        mount_point=mount_point, mount_label=mount_label, **kw)
      File "/nfs/home/dbrondsema/tracwikiimporter/tracwikiimporter/importer.py", line 105, in import_tool
        load_data(f.name, WikiFromTrac.parser(), options)
      File "/nfs/home/dbrondsema/tracwikiimporter/tracwikiimporter/scripts/wiki_from_trac/loaders.py", line 56, in load_data
        import_wiki(cli, options.project, options.wiki, options, doc_txt)
      File "/nfs/home/dbrondsema/tracwikiimporter/tracwikiimporter/scripts/wiki_from_trac/loaders.py", line 77, in import_wiki
        r = cli.call(url + '/' + title, **page)
      File "/home/dbrondsema/dbrondsema-1019/forge/Allura/allura/lib/import_api.py", line 59, in call
        raise e
    HTTPError: HTTP Error 403: Forbidden (http://sf-dbrondsema-1015.sb.sf.net/rest/p/testit/wiki-sivic/osirix_plugin)
    

    Since forum import is not documented or tested (and probably not even used either), I'm not too comfortable with the fact that now any oauth token can use it, rather than the previous situation that required an import token that site admins specifically granted import capabilities to. What do you think about adding back a simple restriction on forum imports? That would mean by default the feature isn't available but a site admin could turn it on if needed. I'm thinking maybe something much simpler than the previous admin web interface. Could be just something like a .ini config option to list bearer tokens and/or projects that are allowed to use it.

     
  • Igor Bondarenko

    Igor Bondarenko - 2014-05-13

    Regarding an exception: seems like importer using http://, instead of https:// (and auth code is checking that). I guess, I missed it, 'cause I commented out that line to be able to test on vagrant image, where https is not configured.

    Created #588: [#1687] Fix tracwikiimporter after #582 (1cp) for this.

    I think restriction for forum import might be added back pretty easily if you're ok with .ini config approach.

    Created #589: [#1687] Add back capability checks for forum import (1cp) for this.

     

    Related

    Tickets: #1687

  • Dave Brondsema

    Dave Brondsema - 2014-05-13

    Sounds good. Whatever's the absolute easiest for a capability check will be fine.

     
  • Igor Bondarenko

    Igor Bondarenko - 2014-05-23

    Closed #588. Updated tracwikiimporter:je/42cc_1687

    d1722ad..3d4e262  je/42cc_1687 -> je/42cc_1687
    
     
  • Igor Bondarenko

    Igor Bondarenko - 2014-05-29

    Closed #589. Updated allura:je/42cc_1687

    8f53750..9d862bb  t589_add_capability_checks -> je/42cc_1687
    
     
  • Igor Bondarenko

    Igor Bondarenko - 2014-05-29
    • status: in-progress --> code-review
     
  • Dave Brondsema

    Dave Brondsema - 2014-06-04
    • status: code-review --> closed
    • Size: 2 --> 0
    • Milestone: forge-backlog --> forge-jun-13
     
  • Dave Brondsema

    Dave Brondsema - 2015-01-05
    • Milestone: unreleased --> asf_release_1.2.0
     

Log in to post a comment.