Support for SSO basic-auth without dialogue. #1268
Conversation
app/login/index.js
Outdated
| if (ssoUser && ssoPasswordCommand) { | ||
| //TODO: Implement a timeout ? | ||
| console.log(`Retrieve password using command : ${ssoPasswordCommand}`); | ||
| exec(ssoPasswordCommand, (error, stdout, stderr) => { |
There was a problem hiding this comment.
Fairly new to nodejs, maybe document if this is running in a shell or not (to be able to use pipes ect in the provided ssoPasswordCommand
There was a problem hiding this comment.
You can simplify this by using execSync. Something like:
| exec(ssoPasswordCommand, (error, stdout, stderr) => { | |
| try { | |
| const ssoPassword = execSync(ssoPasswordCommand).toString(); | |
| callback(ssoUser, ssoPassword); | |
| } catch (error) { | |
| console.error(`Failed to execute ssoPasswordCommand. Status Code: ${error.status} with '${error.message}'`); | |
| } |
I haven't tried the code but that should execute it synchronously and I think that is ok in this case.
lecler-i
changed the title
There was a problem hiding this comment.
After applying the changes, if you can increase the number in the package.json (to 1.4.40) and add a line in the https://github.com/IsmaelMartinez/teams-for-linux/blob/develop/com.github.IsmaelMartinez.teams_for_linux.appdata.xml#L17 with what was added, then I can get it released soon (as a pre-release 1st)
app/config/index.js
Outdated
| describe: 'Command to execute to retrieve password for SSO auth.', | ||
| type: 'string' | ||
| }, | ||
|
|
There was a problem hiding this comment.
I would remove the empty lines in 145 and 156 (maybe is just the editor showing them to me)
app/config/index.js
Outdated
| @@ -142,6 +142,18 @@ function argv(configPath, appVersion) { | |||
| describe: 'Custom Client Certs password for corporate authentication (certificate must be in pkcs12 format)', | |||
| type: 'string' | |||
| }, | |||
|
|
|||
| ssoUser: { | |||
There was a problem hiding this comment.
Could you update the 'REAME.md' file in this folder with this two options? I try to put them in alphabetical order but people tend to ignore that 😄
app/login/index.js
Outdated
| if (ssoUser && ssoPasswordCommand) { | ||
| //TODO: Implement a timeout ? | ||
| console.log(`Retrieve password using command : ${ssoPasswordCommand}`); | ||
| exec(ssoPasswordCommand, (error, stdout, stderr) => { |
There was a problem hiding this comment.
You can simplify this by using execSync. Something like:
| exec(ssoPasswordCommand, (error, stdout, stderr) => { | |
| try { | |
| const ssoPassword = execSync(ssoPasswordCommand).toString(); | |
| callback(ssoUser, ssoPassword); | |
| } catch (error) { | |
| console.error(`Failed to execute ssoPasswordCommand. Status Code: ${error.status} with '${error.message}'`); | |
| } |
I haven't tried the code but that should execute it synchronously and I think that is ok in this case.
|
Thanks for contributing! |
|
Oh yea sorry for the readme,config and version bump, was just a super-quick draft. |
|
sorry, wrong thumps . As it is under a feature flag (config option) I am less worried as it should not change the default use case, but I tend to release it under a pre-release so people can try it before it hits the large amount of users. Thanks again! |
lecler-i
force-pushed
the
feature/sso-login-exec-command-for-password
branch
from
a90f9cb
to
85a4453
Compare
Added two new config keys : `ssoUser`, `ssoPasswordCommand` that will be used instead of the regular login/password
dialogue.
Authentication will be setup with the `login` with content of `ssoUser` key, and the password will be the stdout of the
execution of the command in `ssoPasswordCommand`.
Example of config :
```json
{
"ssoUser": "Thomas",
"ssoPasswordCommand": "gopass -o work/mycompany.com"
}
```
lecler-i
force-pushed
the
feature/sso-login-exec-command-for-password
branch
from
85a4453
to
cc99708
Compare
|
let me know once you are finished and I can create the pre-release. Thanks again! |
|
Ok i've updated README, bumped the version and using But it's good to me, tested and working. |
|
aye, its fine to me also so merging. Thanks again! I will announce the release on the other issue. |
|
Might wanna rename the parameters to "basicAuthLogin" / Password or smthing before release ? |
|
done it in the 1.5.0. Thanks again for the contribution! |
Added two new config keys :
ssoUser,ssoPasswordCommandthat will be used instead of the regular login/password dialogue.Authentication will be setup with the
loginwith content ofssoUserkey, and the password will be the stdout of the execution of the command inssoPasswordCommand.Example of config :
{ "ssoUser": "Thomas", "ssoPasswordCommand": "gopass -o work/mycompany.com" }