sshaw / Mojolicious-Plugin-DigestAuth

HTTP Digest Authentication for Mojolicious
1 stars 5 forks source link

Make build_auth_request() available through a test module #3

Closed Htbaa closed 9 years ago

Htbaa commented 11 years ago

Hi,

I'm currently using your module to quickly add some authentication to a (small) project I'm working on. When I wanted to start writing tests I found out that Mojo::Test (or rather Mojo::UserAgent) doesn't support digest authentication. So testing with this plugin enabled isn't directly possible.

In your TestHelper module the function build_auth_request fixes this issue, but it's not part of the distribution (it's in the test suite).

I was thinking perhaps it would be a good idea to make the build_auth_request function available through some test module so testing can be made easier?

sshaw commented 11 years ago

I never liked the fact that build_auth_request was more or less dup'n what the plugin was doing but, in this case, don't you think it's better to do something like:

sub an_action { 
  my $self = shift;
  return unless $self->app->mode ne 'production' or $self->digest_auth
   # ...
}

# Or maybe set it up in a hook

sub production_mode {
  my $self = shift;
  my $admin = $self->digest_auth('/admin');
  $admin->route('/new')->to('users#new');
}

And if you must test that auth is enabled:

ENV['MOJO_MODE'] = 'production';
Test::Mojo->new(App->new)->get_ok('something')->status_is(401);

?

If anything I think the best thing to do would be to create a ::DigestAuth::Test package that would add the functionality to Test::Mojo... but, on that note, why DigestAuth over Basic + SSL?

Htbaa commented 11 years ago

Personally I don't think it's a good/nice thing to let behavior of a subroutine be influenced that way.

Basic authentication would work for me as well. I haven't really looked into the modules yet that provide that. I've found both Mojolicious::Plugin::BasicAuthPlus and Mojolicious::Plugin::BasicAuth. What I like about your module is that it's possible to authenticate at the router-level, instead of inside the action of a controller.

I'm sure those others can probably manage the same by using a callback in a route or something, but haven't really invested in it yet.

sshaw commented 11 years ago

Hi, I hadn't done anything with Mojo for a while but after playing around with Mojo::UserAgent I see your point about making build_auth_request available.

How about doing something like this, it's really a general solution for digest auth using Mojo::UserAgent:

use Mojo::UserAgent::DigestAuth 'digest_auth';
use Test::Mojo;

# Still uses basic by default
my $t = Test::Mojo->new;
$t->get_ok('http://sshaw:password@example.com')->status_is(200);

# Use digest
$t->get_ok('http://example.com', { Authorization => 'Digest ' . digest_auth('sshaw', 'password') })->status_is(200);

# This will make digest the default
use Mojo::UserAgent::DigestAuth;
use Test::Mojo;

my $t = Test::Mojo->new;
$t->get_ok('http://sshaw:password@example.com')->status_is(200);

# Applies to Mojo::UserAgent too
say Mojo::UserAgent->new->get('http://sshaw:password@example.com')->res->body;

package Mojo::UserAgent::DigestAuth;

use Mojo::Base -strict;
use Mojo::Util 'monkey_patch';
use Mojo::UserAgent;

sub import
{
    if(export digest_auth) {
      # ...
    }
    else {
      monkey_patch 'Mojo::UserAgent', transactor => sub { Mojo::UserAgent::DigestAuth::Transactor->new };
    }
}

sub digest_auth { '->digest<-' }

package Mojo::UserAgent::DigestAuth::Transactor;
use Mojo::Base 'Mojo::UserAgent::Transactor';

sub tx
{
    my $self = shift;
    my $tx   = $self->SUPER::tx(@_);
    my $req  = $tx->req;

    if($req->url->userinfo && !$req->headers->authorization) {
    $req->headers->authorization(Mojo::UserAgent::DigestAuth::digest_auth());
    }

    return $tx;
}

1;
sshaw commented 11 years ago

After giving it some more thought:

 # No override or export
use Mojo::UserAgent::DigestAuth;

# Export digest_auth
use Mojo::UserAgent::DigestAuth 'digest_auth'; 

# Make digest the default
use Mojo::UserAgent::DigestAuth -default
Htbaa commented 11 years ago

I think that would be a nice solution with enough flexibility.

sshaw commented 9 years ago

This fell through the cracks but I see that there is a Mojo::UserAgent::DigestAuth module now available so I'm going to close this.