Skip to content
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

reresubmit: improved persistent cookies :) #31

Merged
merged 15 commits into from
Oct 16, 2012
Merged

reresubmit: improved persistent cookies :) #31

merged 15 commits into from
Oct 16, 2012

Conversation

visit1985
Copy link

After some review rounds with @scroogie I decided to put this into a merge request again.

I used @bartv2's draft and implemented all our comments (from pull request #26).

Here a short summary of the new features:

  • multiple tokens from multiple hosts/browsers can be stored at the same time
  • recreate token after each successfull authentication
  • delete tokens on password change
  • delete tokens older than 15 days (configurable)
  • show a message to the user if cookie authentication fails (pull request #30)
  • use OC_Util::generate_random_bytes() for token generation
  • php session timeout after 1 hour (not configurable jet - I know)
  • php session id regeneration after 15 minutes

Please review and comment!

@scroogie
Copy link

Hi Michael,

it seems to me your generating a 1024 bit key now:

  •    $token =OC_Util::generate_random_bytes(128);
    

Greetings
André

@LukasReschke
Copy link
Member

it seems to me your generating a 1024 bit key
$token =OC_Util::generate_random_bytes(128);

Well, this is probably my fault because the description is not very accurate.
generate_random_bytes() returns a string with the given length (so here 128 chars) using openssl_random_pseudo_bytes().

Let's rename this function to generate_random_string() after merging this.


From a security point of view this should be safe to merge and together with #30 this is a real improvement.

+1 from me.

@visit1985
Copy link
Author

No Lukas, the method name is fine. It was my fault. I mixed up bits and bytes.

As 128byte is no performance problem, we use 256bit / 32byte now. Which is equal to a md5 hash or a PHPSESSID.
I also set the configval column back to varchar(64) because we don't need to extend it ;)

Regards,
Michael

LukasReschke added a commit that referenced this pull request Oct 16, 2012
reresubmit: improved persistent cookies :)
@LukasReschke LukasReschke merged commit 59404b5 into owncloud:master Oct 16, 2012
@LukasReschke
Copy link
Member

Let's merge it - Thanks!

bhawanaprasain pushed a commit to JankariTech/core that referenced this pull request Apr 25, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Aug 19, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants