Mailing List Archive: Catalyst: Users

Patch for Catalyst::Authentication::Store::DBIx::Class::User

Index | Next | Previous | View Threaded

david at olrik

Aug 19, 2008, 5:34 AM

Post #1 of 7 (882 views)
Permalink

Patch for Catalyst::Authentication::Store::DBIx::Class::User

Hi,

When specifying a 'id_field' in the config file as stated in the
documentation it is not used instead of 'username' when looking up
the user.

This means that the select that looks up the user returns a random
user from the database as it has no where clause.

This patch fixed that issue.

Attachments:

User.pm.patch (0.92 KB)

jayk at ion0

Aug 20, 2008, 10:00 PM

Post #2 of 7 (833 views)
Permalink

Re: Patch for Catalyst::Authentication::Store::DBIx::Class::User [In reply to]

Hi David,

Thanks for the patch, but I think you are misunderstanding the use of
id_field.

The id_field config option is only used when saving to / restoring
from the session, it has no relation to what you pass to the $c-
>authenticate() call. By default,
Catalyst::Authentication::Store::DBIx::Class uses the primary key of
the user table. You only need to specify id_field if the primary key
for the table is multi-column.

Your $c->authenticate({ ... } ) call should contain the fields you
want to query on. So if your username field is 'user' then you want
to call:

$c->authenticate({ user => $c->req->params->{'login'}, ... });

Generally speaking, the parameters you pass in to authenticate when
using the DBIx::Class store are the same field names you would use in
a call to $resultset->search({ ... });

Hope that helps!

Jay

On Aug 19, 2008, at 6:34 AM, David Jack Wange Olrik wrote:

> Hi,
>
> When specifying a 'id_field' in the config file as stated in the
> documentation it is not used instead of 'username' when looking up
> the user.
>
> This means that the select that looks up the user returns a random
> user from the database as it has no where clause.
>
> This patch fixed that issue.
>
> <User.pm.patch>
>
>
> --
> Best regards,
> David Jack Olrik <david [at] olrik> http://david.olrik.dk
> GnuPG fingerprint C290 0A4A 0CCC CBA8 2B37 E18D 01D2 F6EF 2E61 9894
> [."The first rule of Perl club is You do not talk about Perl club"]
>
> _______________________________________________
> List: Catalyst [at] lists
> Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst
> Searchable archive: http://www.mail-archive.com/catalyst [at] lists/
> Dev site: http://dev.catalyst.perl.org/

_______________________________________________
List: Catalyst [at] lists
Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst
Searchable archive: http://www.mail-archive.com/catalyst [at] lists/
Dev site: http://dev.catalyst.perl.org/

david at olrik

Aug 21, 2008, 1:12 PM

Post #3 of 7 (834 views)
Permalink

Re: Patch for Catalyst::Authentication::Store::DBIx::Class::User [In reply to]

On 21/08/2008, at 07.00, Jason Kuri wrote:

> Thanks for the patch, but I think you are misunderstanding the use
> of id_field.

Looks like it... I thought id_field worked the same way as
'password_field'.

> Your $c->authenticate({ ... } ) call should contain the fields you
> want to query on. So if your username field is 'user' then you want
> to call:
>
> $c->authenticate({ user => $c->req->params->{'login'}, ... });

Wouldn't it be neat to have the username field configurable, just
like 'password_field' ?

That way you can exchange credential store without changing any
code at all.

--
Best regards,
David Jack Olrik <david [at] olrik> http://david.olrik.dk
GnuPG fingerprint C290 0A4A 0CCC CBA8 2B37 E18D 01D2 F6EF 2E61 9894
[."The first rule of Perl club is You do not talk about Perl club"]

_______________________________________________
List: Catalyst [at] lists
Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst
Searchable archive: http://www.mail-archive.com/catalyst [at] lists/
Dev site: http://dev.catalyst.perl.org/

bogdan at wiz

Aug 21, 2008, 2:31 PM

Post #4 of 7 (832 views)
Permalink

Re: Patch for Catalyst::Authentication::Store::DBIx::Class::User [In reply to]

On Thursday 21 August 2008 23:12:40 David Jack Wange Olrik wrote:
> Wouldn't it be neat to have the username field configurable, just
> like 'password_field' ?

You don't need that, read this:
http://search.cpan.org/perldoc?Catalyst::Authentication::Store::DBIx::Class#Simple_Retrieval

if ($c->authenticate({
username => $c->req->params->{'username'},
password => $c->req->params->{'password'},
status => [ 'registered', 'active', 'loggedin']
})) {

These name => value pairs are used more or less directly in the DBIx::Class'
search() routine, so in most cases, you can use DBIx::Class syntax to retrieve
the user according to whatever rules you have.

Basically you pass whatever hashref you need to $c->authenticate. The
password_field is necessary because of the possible changes done by
Credential::Password .

--
Bogdan Lucaciu
http://www.wiz.ro

dbix-class at trout

Aug 22, 2008, 7:49 AM

Post #5 of 7 (821 views)
Permalink

Re: Patch for Catalyst::Authentication::Store::DBIx::Class::User [In reply to]

On Fri, Aug 22, 2008 at 12:31:20AM +0300, Bogdan Lucaciu wrote:
> On Thursday 21 August 2008 23:12:40 David Jack Wange Olrik wrote:
> > Wouldn't it be neat to have the username field configurable, just
> > like 'password_field' ?
>
>
> You don't need that, read this:
> http://search.cpan.org/perldoc?Catalyst::Authentication::Store::DBIx::Class#Simple_Retrieval
>
> if ($c->authenticate({
> username => $c->req->params->{'username'},
> password => $c->req->params->{'password'},
> status => [ 'registered', 'active', 'loggedin']
> })) {
>
> These name => value pairs are used more or less directly in the DBIx::Class'
> search() routine, so in most cases, you can use DBIx::Class syntax to retrieve
> the user according to whatever rules you have.
>
> Basically you pass whatever hashref you need to $c->authenticate. The
> password_field is necessary because of the possible changes done by
> Credential::Password .

I think the point is that it should be possible to rename fields on the
DBIC side without eneding to change your $c->authenticate line.

--
Matt S Trout Need help with your Catalyst or DBIx::Class project?
Technical Director http://www.shadowcat.co.uk/catalyst/
Shadowcat Systems Ltd. Want a managed development or deployment platform?
http://chainsawblues.vox.com/ http://www.shadowcat.co.uk/servers/

_______________________________________________
List: Catalyst [at] lists
Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst
Searchable archive: http://www.mail-archive.com/catalyst [at] lists/
Dev site: http://dev.catalyst.perl.org/

jayk at ion0

Aug 22, 2008, 8:42 AM

Post #6 of 7 (817 views)
Permalink

Re: Patch for Catalyst::Authentication::Store::DBIx::Class::User [In reply to]

>> On Thursday 21 August 2008 23:12:40 David Jack Wange Olrik wrote:
>>> Wouldn't it be neat to have the username field configurable, just
>>> like 'password_field' ?
>>
>> You don't need that, read this:
>> [ ... ]
>> Basically you pass whatever hashref you need to $c->authenticate. The
>> password_field is necessary because of the possible changes done by
>> Credential::Password .
>
> I think the point is that it should be possible to rename fields on
> the
> DBIC side without eneding to change your $c->authenticate line.

I disagree. The goal seems to be to loosely bind to the DB fields
from the auth side. This should be handled long before the
DBIx::Class store.

The problem is that If we made 'username' configurable, we would have
to make every field configurable. We'd need essentially a map of
'inbound fields' to 'fields to send to ->search()' because username is
only one of the possible ways to search users during authentication.

To do it properly, we'd also have to make a generic way to handle
values and translate them to what would be in the database. Taking
the example from the POD status => 'active' in the auth call would
need to be translatable to a 'user_access_level' field which could
have the values of 'logged in' 'registered' or 'admin'.

There are too many more reasonable ways to accomplish this in an
application that would not be convoluted and would not bury the fact
that it is occuring at all. Adding it at the store level would be
difficult to do well for relatively questionable gain.

Jay

_______________________________________________
List: Catalyst [at] lists
Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst
Searchable archive: http://www.mail-archive.com/catalyst [at] lists/
Dev site: http://dev.catalyst.perl.org/

dbix-class at trout

Aug 25, 2008, 11:38 AM

Post #7 of 7 (771 views)
Permalink

Re: Patch for Catalyst::Authentication::Store::DBIx::Class::User [In reply to]

On Fri, Aug 22, 2008 at 09:42:07AM -0600, Jason Kuri wrote:
> >>On Thursday 21 August 2008 23:12:40 David Jack Wange Olrik wrote:
> >>>Wouldn't it be neat to have the username field configurable, just
> >>>like 'password_field' ?
> >>
> >>You don't need that, read this:
> >>[ ... ]
> >>Basically you pass whatever hashref you need to $c->authenticate. The
> >>password_field is necessary because of the possible changes done by
> >>Credential::Password .
> >
> >I think the point is that it should be possible to rename fields on
> >the
> >DBIC side without eneding to change your $c->authenticate line.
>
> I disagree. The goal seems to be to loosely bind to the DB fields
> from the auth side. This should be handled long before the
> DBIx::Class store.
>
> The problem is that If we made 'username' configurable, we would have
> to make every field configurable. We'd need essentially a map of
> 'inbound fields' to 'fields to send to ->search()' because username is
> only one of the possible ways to search users during authentication.
>
> To do it properly, we'd also have to make a generic way to handle
> values and translate them to what would be in the database. Taking
> the example from the POD status => 'active' in the auth call would
> need to be translatable to a 'user_access_level' field which could
> have the values of 'logged in' 'registered' or 'admin'.
>
> There are too many more reasonable ways to accomplish this in an
> application that would not be convoluted and would not bury the fact
> that it is occuring at all. Adding it at the store level would be
> difficult to do well for relatively questionable gain.

Hmm. Actually, I think having the DBIC level be able to keep the mapping
would probably be better.

Which of course means I need SQLA2 and the DBIC collection API I have
planned for DBICMoose.

Which of course means now it's my problem again.

In the friendliest possible way, fuck you you fucking fuck :P

*crawls off into a corner to dream of enough tuits to get all this done*

--
Matt S Trout Need help with your Catalyst or DBIx::Class project?
Technical Director http://www.shadowcat.co.uk/catalyst/
Shadowcat Systems Ltd. Want a managed development or deployment platform?
http://chainsawblues.vox.com/ http://www.shadowcat.co.uk/servers/

_______________________________________________
List: Catalyst [at] lists
Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst
Searchable archive: http://www.mail-archive.com/catalyst [at] lists/
Dev site: http://dev.catalyst.perl.org/

Index | Next | Previous | View Threaded

Interested in having your list archived? Contact Gossamer Threads