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

add pin-based security for doc requests #262

Merged
merged 2 commits into from Aug 16, 2017

Conversation

@phluid61
Copy link
Contributor

@phluid61 phluid61 commented Aug 15, 2014

When configured (see cfg.d/request_copy.pl), this security model allows authors to respond to copy requests without having a valid login.

The pin-based model allows the contact author to respond whether they have an EPrints account associated with the specified email address or not. The normal repository authentication mechanism is not used, instead the presence of a random pin in the querystring identifies the contact author and grants access.

@jesusbagpuss
Copy link
Contributor

@jesusbagpuss jesusbagpuss commented Sep 10, 2015

Also related to: #192
May need an Admin/Manager screen to allow response to request?

@cziaarm cziaarm added this to the 3.3.15 milestone Sep 10, 2015
@cziaarm
Copy link

@cziaarm cziaarm commented Sep 10, 2015

This is almost "looks good to me".

BUT

  • Install not smooth: Bundled Session::Token ends up missing from @inc despite appearing to be OK on the face of it. Tested the above with pre-installed Session::Token.

@phluid61
Copy link
Contributor Author

@phluid61 phluid61 commented Sep 11, 2015

Yeah, if Session::Token is an issue it should be removed, and instead be listed as an EPrints dependency. I wasn't able to fully test that, since I already have it installed too.

(Feel free to send a PR to our branch if you know of a simple fix)

@MrkGrgsn
Copy link
Contributor

@MrkGrgsn MrkGrgsn commented Sep 11, 2015

I haven't checked it out but EPrints must be generating tokens for sessions somehow; perhaps the Session::Token dependency could be replaced with whatever that is?

@cziaarm
Copy link

@cziaarm cziaarm commented Sep 11, 2015

John (jesusbagpuss) and I were just asking that same question. The "code" param for the token used by the current Request::Repond does this to generate a random string:

my @A = ();
srand;
for(1..16) { push @A, sprintf( "%02X",int rand 256 ); }
my $code = join( "", @A );

This is what DataObj::LoginTicket does:

sub _code
{
my $ctx = Digest::MD5->new;
srand;
$ctx->add( $$, time, rand() );
return $ctx->hexdigest;
}

I suspect that Session::Token does a more robust job than both of these (?), but if those other methods suffice then we would avoid the need to create a dependency.

Better would be to abstract the Token generation (EPrints::Utils perhaps?) and have it done conditionally so that if someone has Session::Token installed Eprints will use that. Otherwise use one of those first-principle methods.

Still invoke it if available, otherwise fall back to regular
rand()-based token generation
@phluid61
Copy link
Contributor Author

@phluid61 phluid61 commented Sep 14, 2015

I updated the PR using Request::Respond's code; if accepted we could update Request::Respond to use the new method in EPrints::Utils too, to keep things DRY.

Maybe even DataObj::LoginTicket.

@denics
Copy link
Contributor

@denics denics commented Aug 1, 2016

Dear all,

this is a great functionality. There is a specific motivation why it has not been merged yet?

@jiadiyao jiadiyao merged commit 5c07ed4 into eprints:3.3 Aug 16, 2017
@MrkGrgsn
Copy link
Contributor

@MrkGrgsn MrkGrgsn commented Aug 16, 2017

Glad to see this finally went in! 👍

@phluid61 phluid61 deleted the 20140815-pin-based-requests branch Aug 16, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

6 participants