Skip to content
This repository has been archived by the owner on Feb 6, 2024. It is now read-only.

Add context parameter for authenticators #126

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

meanphil
Copy link

Hi there,

When logging in my users I also need a parameter for which country they want to log into. Instead of just adding a third parameter called country and keep it as a local branch I thought I'd make it more generic in the hope you'd merge this into CASino?

Anyway, I've added a third parameter to authenticators' #validate method called context, which could just be a Hash or really any data that's specific to the particular authenticator class you're using.

meanphil and others added 3 commits April 7, 2015 14:12
A fresh Rails 4.2 install includes sass-rails 5.0.3, which clashes with CASino's requirement of sass-rails 4.x
Sometimes a login might require more information than just a username and password, in these cases the extra info can be pased to the authenticator via the context parameter as a Hash.
@joelvh
Copy link
Contributor

joelvh commented Sep 26, 2015

@meanphil This is exactly what I just worked on and came up with the same solution. However, I think there needs to be a way to build a context object server-side. I'm still devising a way to do that -- likely a CASino config option to specify a Proc to build a context.

@joelvh
Copy link
Contributor

joelvh commented Sep 26, 2015

@meanphil I've sent a PR (meanphil#1) to your fork so you can review/merge and update this PR

Add configurable context when validating credentials
@@ -23,6 +23,10 @@ def current_user
tgt.user
end

def current_authenticator_context
CASino.config.authenticator_context_builder.call(params, request)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the suggested approach to set CASino.config.authenticator_context_builder?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pencil it's very simple -- your "builder" returns an object that is used by your custom authenticator. In my case, I just need to get the host of the request, so that I can look up specific records segmented by the current hostname (or domain name or subdomain name). In my case, I return a Hash such as {host: request.host}. I could look at headers, request parameters, or other places to get more data about the request that may be relevant to my authenticator (particularly in a multi-tenant setup, where the database is segmented based on something like the domain name).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was more thinking about: How is the config option set? Options are currently loaded from the cas.yml which in this case will not work (since it has to be a Proc). Is the suggested approach to set this specific option in an initializer?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pencil we could have a authenticator_context_builder_class option that is "constantized" and set to the authenticator_context_builder property. The class would simply need to have a call method on it.

Of course, we could create a default constant class that parses config options and does something like refer to request properties, but that gets very complicated and also requires specific opinions about what the context does or consists of. We should keep it to declarative procs or classes.

Thoughts?

@joelvh
Copy link
Contributor

joelvh commented Nov 3, 2015

@pencil can this be merged in?

@joelvh
Copy link
Contributor

joelvh commented May 16, 2017

@pencil can this be merged? Are you still maintaining this project or accepting contributions?

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.

3 participants