Support for multiple accounts under the new authentication scheme #138

Closed
atkinsj opened this Issue Apr 23, 2016 · 9 comments

Projects

None yet

3 participants

@atkinsj
atkinsj commented Apr 23, 2016

This is a big thing missing when moving to centralised CREST SSO. Scouts have multiple accounts and alternate between them: having to logout and relogin constantly is a massive hindrance to usability.

We need a way to associate a Pathfinder login with multiple accounts.

@exodus4d
Owner

Yeah this is indeed a Problem. It took me some time to finish #131 (comment) first :(. All new features are now implemented for v1.0.0RC3. Now I´ll focus on this Issue and some smaller bugs before it is ready to release it.

@exodus4d exodus4d self-assigned this Apr 24, 2016
@exodus4d exodus4d added this to the v 1.0.0RC3 milestone Apr 24, 2016
@atkinsj
atkinsj commented Apr 24, 2016

That's awesome -- thanks again for all the hard work, I hope you don't
think I'm giving you a hard time! I'm passing back feedback from a bunch of
our scouts.

On 24 April 2016 at 11:29, Mark Friedrich notifications@github.com wrote:

Yeah this is indeed a Problem. It took me some time to finish #131
(comment)
#131 (comment)
first :(. All new features are now implemented for v1.0.0RC3. Now I´ll
focus on this Issue and some smaller bugs before it is ready to release it.


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#138 (comment)

@exodus4d
Owner
exodus4d commented May 1, 2016 edited

Oh no, I appreciate our feedback! Unfortunately I don´t have the time to play a lot, therefore user feeback is very important for me!

How it works

  • Any user has to login via CCP´s SSO*, when using *Pathfinder, with a character, for the first time (this should always work on the current develop branch.
  • User gets a sort Cookie notification, which can be accepted (or not). Cookies will not be send to clients that have not accepted this hint. (EU legislation on cookies)
  • If login was successful a login-cookie is send to the client
  • When he comes back, Pathfinder will check for existing cookies and provide direct character login for valid characters.

cookie_login

Technical information

After reading a lot about "long term User authentication" and security concerns, I really liked the way described in this article "Implementing Secure User Authentication in PHP".(awesome article!)

  • Any Cookie is stored on a new DB table character_authentication which is used to verify the validity of each Cookie (Client side manipulation e.g. changing the "expire date" will result in an invalid Cookie,...)

auth_table

  • There is no "blank" data stored on a Cookie (like DB ids,...). So there is no chance of "guessing" other Cookie data. No way to get additional information about the App (e.g. number of users/characters which would be the case when we would use DB ids,...)
  • Expire time can be set in pathfinder.ini (default: 30days)
  • Each character has its own Cookie. The Cookie name is unique ("char_" . md5([CHARACTER_NAME]). The name does not have to be secure, it just should be unique :)
  • The Cookie value of it consists of two parts
    • A unique selector generated out of an random initialization vector which used to identify the validity of the Cookie against the DB
    • A validator which represents the character name. The hashed (sha256) version of this validator is stored in the DB column and is used for an additional layer of security

auth_cookie

  • Even if somebody breaks into the DB, it is not possible to generate valid Cookie information just from the DB tables
  • Cookies are set with httponly = true setcookie(); to protect against JS manipulation.

Cookie verification process

  1. Read all character log in Cookies
  2. Check Cookie format (selectorand validator must be present)
  3. Select DB column from character_authentication table where selector data matches appropriate column
  4. Check if Cookie expire time matches DB expire time in character_authentication
  5. Check if character "exists" is "valid" and "active"
  6. Check if validator column character_authentication table matches sha256-hashed version of Cookie validation data
  7. Try to update Character over CREST (update access_token, e.g. update corp/ally,...)
  8. Check if character is authorized to log in (e.g. corporation has changed/not on "white list", or App access was changed/limited to specific corps/allys,...)
  9. Login :)
@atkinsj
atkinsj commented May 1, 2016

I'm glad the article was helpful; from a security standpoint your implementation looks solid but I'll have a closer look today as it's not currently working for me (possibly because you set the cookie via javascript instead of PHP and I have some addons running).

Would it be possible to set a default character so that when you land on the homepage you get automatically logged in? You could then use the character selector at the top to switch accounts as well.

@atkinsj
atkinsj commented May 1, 2016 edited

Hmm, I might be crazy, but I only see the EU legislation cookie actually being set in your commit? None of the char_ cookies appear to being set at all. I'm also not convinced that the EU legislation cookie is actually still a legal requirement.

EDIT: Nevermind. It was an addon.

The only thing you're missing that I see from a security perspective is the ability to revoke cookies. Clicking the "Logout" menu item should destroy the cookies and remove them from the character_authentications database table. You can destroy a cookie by setting it's expiry in the past as in:

setcookie($cookiename,'',strtotime('-1 year'));

Given that a logon gets you CREST access to the users location, you should add a menu item that destroys all authentication tokens for that character. That way a user can choose to make all the cookies that are out there in the wild (potentially on other computers, Internet cafes, etc) invalid from any current logon session.

@exodus4d
Owner
exodus4d commented May 1, 2016

The first cookie (which is set when you confirm the "set cookie" hint at the bottom) is set by javascript.
The character cookies are set by PHP when you log in by SSO: sso.php#L208 in this function
setLoginCookie();

The characters you see on the homepage are already "verified" and updated by CREST, by checking your cookies. This is done by Ajax requests for each character cookie on document.onload. I implemented it that way, because it takes about 100ms to perform CREST calls and DB updates for each character. -> Otherwise a user with 10 characters would wait about 1s before the page gets initial loaded.
Just click on one of the character images and you are logged in.
But you can still click the SSO button, for log in a fresh character. This will also update cookie data of an existing character you already have access to (e.g. you click the SSO button instead of the character image,..)
I don´t think we need a "default character", it is just one click to select a char.

I´m not completely done with this task. There are still some things missing:

  • When a character was sold -> all existing character_authentication relations must be deleted
  • The getCookieCharacters(); handles all the verification process of character cookies (invalid cookies are deleted at the end). But we need a new cronjob as well which deletes expired cookies (just to keep the DB clean). This should be done on EVE server downtime
  • You are right, a character "logout" should also delete cookie information from DB
  • The character switch (on the map page) is currently not "in line" with the one on the homepage (and not using those cookie information). This will be unified.
@exodus4d
Owner
exodus4d commented May 1, 2016

Because Pathfinder gets more depending on CCPs CREST API by the time. It would be helpful to see/check CREST status and EVE server status information. In case CREST is down, you will not be able to log in and use the app.
Therefore a implemented a small notification panel on the login page:

server_status_panel

@exodus4d exodus4d added a commit that referenced this issue May 2, 2016
@exodus4d - #84, #138 improved "character selection" on login page (expired coo…
…kies are deleted, character panel layout improvements)

- added new "Server info panel" to the login page
- added new cronjob to delete expired cookie authentication data
8900276
@exodus4d exodus4d added a commit that referenced this issue May 6, 2016
@exodus4d - PHP Framework upgrade 3.5.0 -> 3.5.1 (fixes some issues with CREST …
…cURL caching, and SESSION management)

- #138 added "cookie logout" to "logout" menu entry
dfd1e85
@exodus4d
Owner
exodus4d commented May 6, 2016 edited

dfd1e85 Added new "cookie logout" functionality. -> If you manually logout (click on the menu entry), all server-side stored cookie information are deleted, too. If you get automatically logged out (e.g. on server errors), your login cookies are still active.

  • This will prevent anyone else to login this character with a valid cookie
  • This will delete your cookie for this character
  • This will not logout any character from the active user

If there is already someone else logged in (with an active SESSION), he will logged out as soon as PATHFINDER.TIMER.LOGGED (default: 240 minutes -> pathfinder.ini) expires. This may can be improved in the future but requires some additional programming to work.

@mrurb
mrurb commented May 29, 2016

I woulds suggest making the Accept cookies notice a bit bigger and more obvious.

@exodus4d exodus4d referenced this issue Jun 3, 2016
Merged

v1.0.0 #183

@exodus4d exodus4d added a commit that closed this issue Jun 3, 2016
@exodus4d v1.0.0 (#183)
* #84 test data dump from CREST login

* updated "credits" dialog (Google+ link)
fixed login form layout

* updated Cortex Data-Mapper

* - #84 CREST Login (WIP)
- New CREST controller
- Database restructuring
- improved type-casting for some controller functions
- New login process
- Fixed some bugs during the setup process (/setup root)
- Added CREST request caching by response headers

* pathfinder-84 [Feature Request] CREST Pilot Tracking, many smaller Bugfixes

* pathfinder-84 [Feature Request] added develop JS files

* closed #121 fixed wormhole signature type caching

* closed #120 removed map-loading animation for larger maps (same behaviour as IGB)

* closed #119 fixed wormhole signature id count

* closed #114 Added check for already existing system when adding a new one. (fixed PDO 'duplicate entry' error)

* closed #112 fixed DataTables error for missing "status" data (signature table)

* closed #111 fixed convertDataToUTC(); client side date transformation

* closed #109 fixed system TrueSec rounding

* closed #103 fixed system updated timestamp in getData()

* fixed CSS class for secStatus in Routes module

* closed #121 fixed wormhole signature type caching

* changed dateTime format from German to US format
fixed some minor bugs in signatureTable module

* closed #81 fixed "signature type" overwriting by "signature reader" update

* closed #106 added new signature_types form C5/6 wormholes (gas/ore)

* closed #129 fixed parameter hinting

* closed #131 new "route search" algorithm, added current map systems to live search, added refresh/update functionality for each found route, added bulk route refresh function, added "meta map" route search (search on multiple maps), added route "filters" (restrict search on "stargates", "wormholes", "jumpbridges"), added route "filter" for wormholes (reduced/critical wormholes)
closed #89 fixed "loop connections" on same system
#84 added error messages for "invalid" CREST "Client ID"
added "bootboxjs" (customized styled checkboxes/radio buttons) CSS only
"Font Awesome" version upgrade 4.4.0 -> 4.61
"Bootbox.js" version upgrade 4.3.0 -> 4.4.0
fixed "system dialog" (added responsive layout)

* closed #134  fixed db column type DT_INT (8 bytes) to DT_BIGINT

* closed #138 added new cookie based login

* closed #137 fixed javascript errors on trying to establish an "invalid" connection

* - #84, #138 improved "character selection" on login page (expired cookies are deleted, character panel layout improvements)
- added new "Server info panel" to the login page
- added new cronjob to delete expired cookie authentication data

* #138 enables character switching between characters which have same user

* - PHP Framework upgrade 3.5.0 -> 3.5.1 (fixes some issues with CREST cURL caching, and SESSION management)
- #138 added "cookie logout" to "logout" menu entry

* - updated "feature page" with new feature descriptions and label
- added some new images to the "feature gallery"
- removed "beta" status from "magnetizing" feature on map menu
- hide "server status" panel on "mobile" breakpoint

* - #138 clear character authentication data on sold characters

* closed #142 added custom "onsuspect()" session handler

* #142 do not log suspect if no file is defined in pathfinder.ini

* #142 added NullSec Data/Relic sites to C1/2/3 wormholes as signature option

* #144 fixed "Character not found" warning

* #144 fixed "Character not found" warning

* closed #144 fixed broken routes panel in IGB

* updated README.md for upcoming release

* #147 response header validation

* #149 changed comment for 'BASE' framework var

* fixed map  import

* - added minimal SDE dump (EVE Online: Citadel)
- #147 improved CREST API error logging (WIP)
- improved SSO controller (removed access_token from public endpoints)

* closed #154 added alliance maps to CREST API

* - updated Gulp build dependencies
- increased CREST timeout from 3s -> 4s
- added "Accept" Headers for some CREST endpoints

* cloased #147

* - closed #153 added character verification check for getAll(); Signatures Ajax endpoint

* - updated README.md (added Slack developer chat information)

* Bugfix frig holes (#159)

* added missing frigate wormholes and fixed Q003 destination in shattered wormholes

* changed C7 to 0.0 for Q003

* - fixed broken "graph" data for system

* added a  "failover" system  for bad crest requests (HTTP status 5xx,.. )

* Red Gaint => Red Giant (#161)

* closed #163 added CREST endpoint support for "waypoints"

* fixed typo

* closed #160 fixed tooltip container

* - added new features to login page

* closes #154 added alliance map support

* fixed XML path for cronjobs

* fixed a bug with inactive "private" maps

* closes #175 added alternative environment configuration

* - v1.0.0  build
ecd505a
@exodus4d exodus4d closed this in ecd505a Jun 3, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment