Rjm/remoteuser

[richmarisa]

Description / context for this PR:

The remote user variable set by the Apache shibboleth module differs from instance to instance depending on
how PHP was installed. On some instances the variable is REMOTE_USER, on others it is REDIRECT_REMOTE_USER
or other variants. Instead of hardcoding this value, a configuration parameter remote_user_variable is used
to contain the variable name. This is defaulted to REMOTE_USER.

In project code, use env(config('starterkit.remote_user_variable')) to retrieve the netid.

To Review:

Set variables in the .env file
REMOTE_USER_VARIABLE=YOURVARIABLE
YOURVARIABLE=ewe2

• (optionally) Publish the configuration file using the instructions added to the README file
• Verify (e.g., using artisan tinker) that you can retrieve the user netid using the expression above.

[inaydich]

what do you think about renaming the variable to just 'remote_user'?

[inaydich]

I think this PR is a great idea. I just suggest to name it 'remote_user' instead of 'remote_user_variable'. I think 'remote_user_variable' is too long.

[richmarisa]

I worried that it would be too confusing, but as long as people used env(config('starterkit.remote_user')) it would be ok. Thoughts, @woodseowl ?

[woodseowl]

I think the long name is appropriate. remote_user as a name suggests to me that it is some kind of entity.

[inaydich]

I think '_variable' does not give any clue that it is local. Maybe 'remote_user_app' then?
Another question: do we need a separate "starterkit" config file? Would it be cleaner to use config/app.php. It is almost empty. We are using app.js and app.css for other application customizations.

[richmarisa]

The value is the name of the environment variable used by the server (Apache module / PHP), so it really is a variable. We could call it remote_user_environment_variable to be most clear. I think that @woodsowl's idea of putting this in its own package with the middleware is a good one.

[richmarisa]

Since this is in a package we don't have access to config/app.php. starterkit is the shortname of the package, so that is what the config fille is called. If we push this into it's own cu-auth package as Eric suggests, then it can go in cu-auth.php (or cuauth.php? I'm not sure what happens to the dash in the package template).

[inaydich]

I guess we all agree about location: config/cu-auth.php

[woodseowl]

Great to see this in a PR!

I think that env(config('starterkit.remote_user_variable')) is a very specific piece of code and requires knowledge of the internals to know that it is there and how to use it. That makes me think we need to wrap it with more context somehow.

Looking at how we use that environment value in another project, I see a CUAuth middleware that picks it up and registers and/or logs in the user along with storing the user in the session. I presume that's all we are doing with this? What if we made a mini package called cornell-custom-dev/cu-web-auth which initially has two things:

• config/cu-auth.php
• CornellCustomDev/CUAuth/Http/Middleware/CUAuth

The config file would be exactly what you have in this PR, but it would then be referenced as config('cu-auth.remote_user_variable'), which I think makes sense as a variable name. The middleware could have Laravel Authorization integration with CUAuth (or do something basic that doesn't use the db, like manage a session variable).

If we do that, would anything outside that package even need to know what remote_user_variable is? Hopefully not, because it's entirely encapsulated behind the functionality that is what we actually want.

[inaydich]

I like using config/cu-auth.php for this specific setting. I still do not like using "_variable". I do not see any value in it. Also pls see my other comment about app config

[richmarisa]

Putting this in it's own package with the middleware makes sense, but note that the .env file in the server instance is still going to need a key called REMOTE_USER_VARIABLE or REMOTE_USER_ENVIRONMENT_VARIABLE.

[inaydich]

REMOTE_USER_ENV

what about REMOTE_USER_ENV?

[woodseowl]

Putting this in it's own package with the middleware makes sense, but note that the .env file in the server instance is still going to need a key called REMOTE_USER_VARIABLE or REMOTE_USER_ENVIRONMENT_VARIABLE.

Isn't it that you set the default value in the config file and then REMOTE_USER_VARIABLE is only ever set directly in .env if a dev or test instance isn't the same as prod?

[inaydich]

right, we'll only need to set that variable in a local .env file

[richmarisa]

The .env key is not necessary on local development, but would be used on M3 servers if the NetID is supplied in an environment variable other than REMOTE_USER. There are installation differences on the servers such that sometimes the NetID is in REDIRECT_REMOTE_USER, or something else.

[richmarisa]

@woodsowl if a cu-auth package includes middleware, it is going to depend on a User model which contains a NetID field. Where do you see that being included?

[woodseowl]

.env key: If the default value is set in config/cu-auth.php in the installed project, then you just configure the default value in that project's config/cu-auth.php to be what it should be on production for that project. Then if any other version of that site (dev/test/local) needs a different value you can define the variable name in .env.

User model: It's in the base Laravel install, so all we need is a migration to add users.netid. I would presume we add that as a migration to cu-auth.