Creates yaml client config

[EthanHeilman]

Creates a yaml config file for the opkssh client which lives at ~/.opk/config.yml.

You can create it by running opkssh login --create-config.

Design

Format

---
default_provider: webchooser

providers:
  - alias: google
    issuer: https://accounts.google.com
    client_id: 206584157355-7cbe4s640tvm7naoludob4ut1emii7sf.apps.googleusercontent.com
    client_secret: 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
    scopes: openid email profile
    access_type: offline
    prompt: consent
    redirect_uris:
      - http://localhost:3000/login-callback
      - http://localhost:10001/login-callback
      - http://localhost:11110/login-callback

  - alias: azure microsoft
    issuer: https://login.microsoftonline.com/9188040d-6c67-4c5b-b112-36a304b66dad/v2.0
    client_id: 096ce0a3-5e72-4da8-9c86-12924b294a01
    scopes: openid profile email offline_access
    access_type: offline
    prompt: consent
    redirect_uris:
      - http://localhost:3000/login-callback
      - http://localhost:10001/login-callback
      - http://localhost:11110/login-callback
      
  - alias: okta
    issuer: https://subdomain.okta.com
    client_secret: 03015ee2ff6e17d61546e061b440eebb
    scopes: openid email profile groups

There is a set of default values allowing the config to be shorter unless those values are needed - suggested by @yonatan-sevenai

When we wish to add new types of providers such as workflow providers like github actions and gitlab-ci they will have their own provider list.

providers:
 - alias: google
    issuer: https://accounts.google.com
    client_id: 206584157355-7cbe4s640tvm7naoludob4ut1emii7sf.apps.googleusercontent.com
    ...
workflow_providers:
  - alias: github
    issuer: https://token.actions.githubusercontent.com
    audience_prefix: "opkssh:"

Why not XDG Basedir

I had originally planned to XDG Basedir standard for config files as provided by https://pkg.go.dev/os#UserConfigDir for XDG Basedir and proposed in #94

I opted not to use XDG Basedir for the config. Instead using ~/.opk/config.yml for all OSes. My reasons are as follows:

XDG Basedir Config uses a different path for each OS

Windows: ~\AppData\Roaming\.opk\config.yml

OSX: ~/Library/Application\ Support/.opk/config.yml

Linux: ~/.config/.opk/config.yml

This means that anytime I am debugging an issue with a user I will need to first determine what OS they are running. Additionally this longer path means they will likely type it wrong when trying to find the config. This is especially true with OSX where the path as an space in it!!! Nothing makes debugging worse than having to ensure that someone escapes a space in a path

This config file should be easily found and edited by the user

XDG Basedir Config is a long path which is hard to type correctly and hard to remember. Is it ~\AppData\Local or ~\AppData\Roaming? On windows and OSX it isn't very discoverable.

No one respects XDG Basedir Config (a standard that isn't standard)

On windows, microsoft's vscode uses ~\.vscode not ~\AppData\Roaming\.vscode, microsofts dotnet uses ~\.dotnet. Microsoft does not even follow the XDG basedir config path that microsoft choose.

On OSX there is a similar pattern

• rust is ~/.rustup
• cargo is ~/.cargo
• git is ~/.gitconfig
• nvm is ~/.nvm
• npm is ~/.npm
• vscode is ~/.vscode
• bash is ~/.bashrc
• golang ~/go (when installed by brew)

Open questions

Should we automatically create the config on first login?

I had originally been against this idea, but given time I've come around to it. This PR allows a user to create a config by typing opkssh login --create-config but it does not automatically create the config.

TODOs

• Parse yaml config
• Use in login.go
• Figure out the plan if this file should be automatically created or not
• Create docs
• Create config argument
• Unittests

Deferred TODOs

• CLI commands to manage this file and detect errors - Create CLI for auditing client config and finding config errors #153

Tests

Added many unittests to lock down this feature. Also manually tested the config, by editing it and seeing behavior change.

[markafarrell]

Would you consider something similar to what I proposed for the server config.

A single file per provider
in ~/.opkssh/providers.d/

with default_provider in ~/.opkssh/providers.yml

It would make it easy to add and remove providers rather than having to edit the client config file.

[EthanHeilman]

@markafarrell Currently I'm leaning toward having one client config file on the client and a directory on the server, but I want to understand your usecase for having a directory on the client. It is important to get this right today.

My current thoughts are as follows (apologies for the wall of text):

• Corruption of the client config is low impact, corruption of the providers on the server has very high impact: If a user corrupts their local client config the damage is minimal, they can just fix it. We should have tooling to help users find errors in the client config, but it isn't the end of the world. The providers config on the server is a different story because if you don't have another way onto that machine and you mess up the providers config you lock yourself out at the next login. This really argues for a strong firewall between providers on the server providers config.

  • The one argument against this is that users are likely to muck about in the client config and try things. Having all the config packaged together makes it more likely they will break a provider.

• Lower cognitive burden: A single client config lets you manage more than just providers but other options as well, such as if auto-refresh is turned on, additional authentication steps, where ssh keys are stored etc... Having a single file to manage all the config for the client has a number of advantages:

  • We need a config file for other things as well as providers so it is nice to just have one.
  • Users have not been trained to think about directories config in the dir.d/ pattern. Many users will not immediately understand that all of these files are included regardless of name.
  • The user learns the location of the file the first time they need to edit it and sees the other config values they can change
  • It provides a single place for a user to look when something goes wrong.
  • Easier to debug over email by saying "Send me the file at ~/.opk/config.yml" is much easier than "send me all the files at ~/.opk/user-settings.yml and ~/.opk/providers.d/*".
  • Consider the hard to find bugs if two client config provider files exist for the same provider or use the same alias. The user only sends the provider file they think is breaking.

• Easy setup: If is far easier for an enterprise or user to download a single config file to everyone's machine then to manage a directory of files.

  • In most circumstances copying a single file is atomic, if the wifi breaks halfway through the download the download doesn't happen and the user can try again. If wifi breaks halfway through a download of multiple files in multiple directories, it can mostly work but then sometimes not work because a particular provider is missing. Or someone updates one provider but not another provider.
  • Asking all your users to update a single file is hard, asking them to update many files is difficult.

Most of these problems are not an issue on the server and fault isolation on the server is much more important.

[markafarrell]

@EthanHeilman

To address:

• Users have not been trained to think about directories config in the dir.d/ pattern. Many users will not immediately understand that all of these files are included regardless of name.

• The user learns the location of the file the first time they need to edit it and sees the other config values they can change

I was thinking we could do something like:

Add a new provider

opkssh provider add google \
  --issuer=https://accounts.google.com \
  --client_id=206584157355-7cbe4s640tvm7naoludob4ut1emii7sf.apps.googleusercontent.com \
  --client_secret=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 \
  --scopes="openid email profile" \
  --access_type=offline \
  --prompt=consent \
  --redirect_uri=http://localhost:3000/login-callback \
  --redirect-uri=http://localhost:10001/login-callback \
  --redirect-uri=http://localhost:11110/login-callback

This creates or replaces ~/.okpssh/providers.d/google.yml with:

google:
  issuer: https://accounts.google.com
  client_id: 206584157355-7cbe4s640tvm7naoludob4ut1emii7sf.apps.googleusercontent.com
  client_secret: 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
  scopes: openid email profile
  access_type: offline
  prompt: consent
  redirect_uris:
    - http://localhost:3000/login-callback
    - http://localhost:10001/login-callback
    - http://localhost:11110/login-callback

Remove provider

opkssh provider delete google

This removes the file ~/.opkssh/providers.d/google.yml

Edit provider

opkssh provider edit google

This opens ~/.opkssh/providers.d/google.yml in the default editor

Modify provider

opkssh provider edit google --set scopes="openid email"

This modifies ~/.opkssh/providers.d/google.yml to set google.scope to openid email (other fields are left unmodified)

Having the primary method of interacting with the provider config be opkssh itself allows us to do more sanity checking on inputs and also ensures that what is stored is valid yaml (protecting new users).

[markafarrell]

Another feature I think would be really nice would be an opkssh test bob [email protected] google command which would do the following.

1. Do an opkssh login and write keys to a tmp location
2. Do an opkssh add bob [email protected] google but write to a temp auth_id instead of the real one.
3. Run opkssh verify using temp keys auth_id and client provider config as the server provider config.

This would essentially enable you to test with the client ONLY if a login would work.

This would only be possible if we kept the provider config for server and client the same

[EthanHeilman]

@markafarrell Would love to see a opkssh test PR

Having the primary method of interacting with the provider config be opkssh itself allows us to do more sanity checking on inputs and also ensures that what is stored is valid yaml (protecting new users).

This makes sense on the server because errors are expensive and bad but the client the user just opkssh login and should get a useful error message.

Typing out a giant command like that is much more likely to result in an error. Opening the yaml file and editing the yaml is a better experience.

opkssh provider edit google this is a neat idea. Would it work well on windows?

[yonatan-sevenai]

My 2c, I think opkssh users will be okay with a providers.d folder, this also makes it play nice(r) with MDM solutions that would allow MDM users to distribute their opkssh providers config to users centrally, which makes for nice ergonomics in enterprise settings.
One critical key is proper default values. I'd love to minimally support a yaml file with

google:
 issuer: https://accounts.google.com
 client_id: 206584157355-7cbe4s640tvm7naoludob4ut1emii7sf.apps.googleusercontent.com
 client_secret: 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

or

okta:
  issuer: https://subdomain.okta.com
  client_secret: 03015ee2ff6e17d61546e061b440eebb
  scopes: openid email profile groups

One small source of confusion would be the name of the file vs the entry in the yaml.
Should we consider the name of the file as the alias, and the content of the file to be just the provider context (not under a google key?)

[EthanHeilman]

@yonatan-sevenai I'm open to adding a providers.d on the client at a point in the future, but right now want things to be as simple as possible. Having the providers in the client config does not lock out a providers.d strategy since we can always just have the providers.d extend the providers in the client config allowing backwards compatibility.

[Copilot]

Pull Request Overview

This PR adds a YAML-based client configuration mechanism and integrates it into the login flow while introducing flags to optionally create the default config file automatically.

• Introduces new flags (config-path and create-config) and updates login command behavior to load or create a config file.
• Refactors provider configuration parsing, loading defaults, and error handling for duplicate/provider alias issues.
• Adds comprehensive tests for client configuration and provider config parsing.

Reviewed Changes

Copilot reviewed 10 out of 11 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
main_test.go	Updated expected error messages in output diff.
main.go	Introduced new flags for config path and creation of a default config.
commands/login.go	Updated login flow to load or create a YAML client config file.
commands/login_test.go	Updated tests to use the new config parameters and updated chooser output.
commands/client-config/*	New implementation and tests for provider config parsing and client config.

Files not reviewed (1)

• go.mod: Language not supported

[Copilot]

Pull Request Overview

This PR introduces a YAML client configuration mechanism for opkssh and integrates it into the login command. Key changes include:

• Adding flags for specifying the config path and auto-creating the config file.
• Implementing functions to parse, validate, and load the client config file as well as provider configurations.
• Updating tests and documentation to reflect the new config file usage.

Reviewed Changes

Copilot reviewed 11 out of 12 changed files in this pull request and generated 1 comment.

File	Description
main.go, commands/login.go	Updated login command to support config file path and creation via new flags.
commands/client-config/*	New code to parse and validate YAML config and provider configurations.
main_test.go, login_test.go	Updated tests to match new configuration-based provider initialization.
README.md	Updated documentation to include instructions on using the new client config file.

Files not reviewed (1)

• go.mod: Language not supported

Comments suppressed due to low confidence (1)

README.md:127

• The documentation contains a typo. It should read 'can be configured' instead of 'can configured'.

This alias to provider mapping can configured using the OPKSSH_PROVIDERS environment variables.