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

Implement script strategy #117

Closed
nichtich opened this issue Sep 11, 2024 · 4 comments
Closed

Implement script strategy #117

nichtich opened this issue Sep 11, 2024 · 4 comments
Milestone

Comments

@nichtich
Copy link
Member

nichtich commented Sep 11, 2024

Strategy to ask for username and password to be passed to a local executable script/binary in the server file system. Requires option command to call the script, e.g. ["/some/login/script", "-u", "$USERNAME", "-p", "$PASSWORD"]. or ["/path/to/script"]. The script gets username and passwort in environment variables USERNAME and PASSWORD and as arguments for array elements being $USERNAME or $PASSWORD, respectively. The script must return a JSON object and not return an error exit code. Field names of the JSON strict are not defined yet (likely id (required account id), name (optional account name) and/or username (optional display name)). A user uri can be generated from template of provider configuration like for other strategies.

@nichtich
Copy link
Member Author

As described at #80 this strategy is going to be used with CBS/PSI in two use cases:

  • individual users (username determines user)
  • group accounts for libraries, user with multiple librarian credentials, e.g.
    "id":"DE-7", "name":" "SUB Göttingen" => "uri":"http://uri.gbv.de/organization/isil/DE-7"

@nichtich nichtich added this to the 1.0.0 milestone Sep 11, 2024
stefandesu added a commit that referenced this issue Sep 13, 2024
@stefandesu
Copy link
Member

Just committed the first implementation. We still need documentation, in particular:

  • How to set it up (easy)
  • Requirements for the script (needs to be able to run on the server environment OR inside the Docker container if Docker is used, i.e. in the latter case, only Bash and Node.js can be used)
  • Maybe a better example

Also I have more questions regarding the implementation:

  • What do you mean by "and as arguments for array elements being $USERNAME or $PASSWORD, respectively"? In particular the part about array elements. (In the current implementation, only environment variables USERNAME and PASSWORD are used.)
  • Usually name or displayName are used for the display name. If neither is given, "username@provider" is used as a fallback. So we need to decide whether to use name or displayName (or we can allow both options for more flexibility). Also we don't normally need username if it's not different from id.
  • In what format are errors being returned from the script? Currently, if no id is given, it is assumed that validation of credentials failed.

@nichtich
Copy link
Member Author

What do you mean by "and as arguments for array elements being $USERNAME or $PASSWORD, respectively"?

Well, passing the credentials per environment should be enough, so ignore this.

I don't get the difference between name and displayName and why we have username at all.

Absence or empty field id is an error as implemented.

@stefandesu
Copy link
Member

Well, passing the credentials per environment should be enough, so ignore this.

👍

difference between name and displayName

There is no difference. Some OAuth providers use one, some use the other. We can just use name. (I can imagine that some providers offer to set both a full name (name) and a separate display name (displayName).)

and why we have username at all.

Because for some providers, the id is a random number, but we still need the username to build the URI (I think this is the case with GitHub, for example).

Absence or empty field id is an error as implemented.

For now, I'm just going to assume that any output where id is empty or not set is a failed authentication, and not differentiate between WHY something failed. I don't think we currently have a mechanism to report those reasons to the user anyway, it'll just show that authentication has failed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants