New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pluggable auth #24189

Merged
merged 26 commits into from May 11, 2016

Conversation

Projects
None yet
7 participants
@ChristophWurst
Contributor

ChristophWurst commented Apr 22, 2016

This revives #1975

I removed the two-factor code from this PR to keep it simple.

TODO:

  • Create session token for new browser session (login)
    • Save hashed token + encrypted login password
  • Invalidate (delete) session token on logout
  • Add background job to kill old session tokens
    • Don't kill permanent client tokens
  • Check browser session on every request -> logout if invalidated/not found
  • Check login credentials each x minutes and log the user out if the check fails
  • Update token 'last_activity' in every request
  • Invalidate tokens if the session is killed in base.php? -> #24542
  • Add controller where clients can get a new access token
    • Can be tested with curl --request POST -F 'user=youruser' -F 'password=yourpassword' localhost:8080/token/generate
  • Don't use sessions if the client sends an auth token -> #24543
  • Add DB index on 'last_activity'
  • Add token type column
  • Migrate OCS auth
  • Adjust sabre auth backend
  • Increase version number to trigger db migration
  • Fix existing unit tests
  • Add new unit tests for token auth
  • Fix existing integration test
  • Fix apache auth (tested with shibboleth sso)
  • Show login errors
  • Add new integration tests for token auth
  • Squash/arrange commits

BUGS:

  • setup is broken
  • user agent is always undefined, so the default is used
  • new user can not log in
  • Changing own password kicks you out, hence the session token needs to be updated on password change -> #24544
Exception: {"Exception":"OCP\\Files\\NotFoundException","Message":"","Code":0,"Trace":"#0 \/home\/christoph\/workspace\/owncloud\/apps\/files\/controller\/viewcontroller.php(118): OC_Helper::getStorageInfo('\/', false)\n#1 \/home\/christoph\/workspace\/owncloud\/apps\/files\/controller\/viewcontroller.php(180): OCA\\Files\\Controller\\ViewController->getStorageInfo()\n#2 [internal function]: OCA\\Files\\Controller\\ViewController->index('', '')\n#3 \/home\/christoph\/workspace\/owncloud\/lib\/private\/AppFramework\/Http\/Dispatcher.php(159): call_user_func_array(Array, Array)\n#4 \/home\/christoph\/workspace\/owncloud\/lib\/private\/AppFramework\/Http\/Dispatcher.php(89): OC\\AppFramework\\Http\\Dispatcher->executeController(Object(OCA\\Files\\Controller\\ViewController), 'index')\n#5 \/home\/christoph\/workspace\/owncloud\/lib\/private\/AppFramework\/App.php(110): OC\\AppFramework\\Http\\Dispatcher->dispatch(Object(OCA\\Files\\Controller\\ViewController), 'index')\n#6 \/home\/christoph\/workspace\/owncloud\/lib\/private\/AppFramework\/Routing\/RouteActionHandler.php(45): OC\\AppFramework\\App::main('OCA\\\\Files\\\\Contr...', 'index', Object(OC\\AppFramework\\DependencyInjection\\DIContainer), Array)\n#7 [internal function]: OC\\AppFramework\\Routing\\RouteActionHandler->__invoke(Array)\n#8 \/home\/christoph\/workspace\/owncloud\/lib\/private\/route\/router.php(280): call_user_func(Object(OC\\AppFramework\\Routing\\RouteActionHandler), Array)\n#9 \/home\/christoph\/workspace\/owncloud\/lib\/base.php(871): OC\\Route\\Router->match('\/apps\/files\/')\n#10 \/home\/christoph\/workspace\/owncloud\/index.php(39): OC::handleRequest()\n#11 {main}","File":"\/home\/christoph\/workspace\/owncloud\/lib\/private\/helper.php","Line":605}
  • Not possible to add an account to the desktop client – I suspect the login url change to cause that. @LukasReschke what are we gonna do about that? It works, I was testing with wrong credentials 🙈
  • "Unable to generate a URL for the named route \\\"login#showLoginForm\\\" as such route does not exist.
  • Adjust column width
    ORA-12899: value too large for column "AUTOTEST"."oc_authtoken"."token" (actual: 128, maximum: 100)
  • username !== uid, #24189 (comment)
  • Login with email doesnt work

Next steps (follow-up PRs):

  • GUI
  • Clients should switch to token auth if the server supports it
  • Bring back remember me, #24189 (comment)

@ChristophWurst ChristophWurst added this to the 9.1-current milestone Apr 22, 2016

@ChristophWurst ChristophWurst changed the title from Pluggable auth to Pluggable auth / 2FA Apr 22, 2016

@ChristophWurst ChristophWurst changed the title from Pluggable auth / 2FA to Pluggable auth Apr 25, 2016

Show outdated Hide outdated db_structure.xml

ChristophWurst added some commits Apr 27, 2016

increase token column width
add some range to time() assertions

@DeepDiver1975 DeepDiver1975 merged commit efa545f into master May 11, 2016

5 of 6 checks passed

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
Scrutinizer 44 new issues, 49 updated code elements
Details
cla-bot-core Build #3957 succeeded in 1 min 29 sec
Details
core-ci-linux-jsunit/database=sqlite,label=SLAVE Build #61866 succeeded in 4 min 40 sec
Details
server-master-linux-externals-smb-windows-ext-ci/database=sqlite,external=smb-windows,label=master Build #14805 succeeded in 3 min 21 sec
Details
server-master-linux-php5.4-ci/database=sqlite,label=SLAVE Build #3572 succeeded in 6 min 31 sec
Details

@DeepDiver1975 DeepDiver1975 deleted the pluggable-auth branch May 11, 2016

@MorrisJobke MorrisJobke referenced this pull request May 13, 2016

Closed

Track user sessions #21592

0 of 3 tasks complete
@guruz

This comment has been minimized.

Show comment
Hide comment
@guruz

guruz May 18, 2016

Contributor

Clients should switch to token auth if the server supports it

@dragotin @danimo How does this relate to the discussions you had yesterday?

Contributor

guruz commented May 18, 2016

Clients should switch to token auth if the server supports it

@dragotin @danimo How does this relate to the discussions you had yesterday?

@ghost ghost referenced this pull request Apr 23, 2017

Merged

Do not register the service 'Session' twice #27724

1 of 9 tasks complete

@DeepDiver1975 DeepDiver1975 referenced this pull request May 18, 2018

Merged

Save timezone as given during login #31468

4 of 9 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment