Skip to content

feat: allow explicitly setting the credentials#100

Open
gpoulin wants to merge 1 commit intoabdolence:masterfrom
gpoulin:overwrite_credentials
Open

feat: allow explicitly setting the credentials#100
gpoulin wants to merge 1 commit intoabdolence:masterfrom
gpoulin:overwrite_credentials

Conversation

@gpoulin
Copy link

@gpoulin gpoulin commented Jun 16, 2025

Currently the plugin allow to overwrite the google credentials file, however it doesn't allow to setup custom credentials that is not tied to a file.

@abdolence
Copy link
Owner

Thanks for the contribution, but I'm sorry this can't be merged since it is intentional design to separate sensitive keys from sbt files. With this design private keys can leak easily to the source code repositories, etc - so it is not recommended and shouldn't be designed this way.

@gpoulin
Copy link
Author

gpoulin commented Jun 26, 2025

The credentials can be build in various way that don't involve putting the credentials in an sbt file. The use case we have is to be able to reload the credentials when the default application credentials file is updated. Our company has a policy that limit the user credentials lifetime, requiring dev to re-login the default application credentials often. At the moment, they also need to restart any sbt process so the credentials can be updated. This restart of sbt could be avoided if the Credentials supported reloading the file when modified. There's also other use case that this could serve. For example, someone could implement credential base on workload identity federation where the federated credentials are in a file and need to be exchanged for a Google credentials. I think those kind of concern shouldn't be part of this plugin, but this plugin should allow supporting those concern by allowing the consumer to build the credentials the way they want.

@abdolence
Copy link
Owner

abdolence commented Jun 26, 2025

I can't see how this change fixes the hot reload issue, since Credentials object will be still cached in HTTP clients and factories from Google.
The hot reload issue that something I can understand but to tackle that issue perhaps it needs another solution so it recreates clients making sure Credentials are reloaded caching only those for some time. I'll look into how this can be supported with different changes.

I think those kind of concern shouldn't be part of this plugin

I disagree, secure by design should be the strategy for all tooling, including sbt plugins.

@gpoulin
Copy link
Author

gpoulin commented Jun 26, 2025

The Credentials is just an abstract class with multiple implementations. This is the abstraction that all GCP clients work with which allow to decouple the authentication implementation from the client logic.

For the reloading Credentials, we simply need to implement the Credentials that is backed by the application default credentials, but reload the underlying Credentials when needed. A likely over-simplify approach would look like

class ReloadingCredentials(file: File) extends Credentials {
  private[this] def buildCredentials = GoogleCredentials.fromStream(new FileInputStream(file))
  private[this] var lastModified: Long = 0L
  private[this] var underlying: Credentials = buildCredentials

  private[this] def reloadIfNeeded(): Unit = if (file.lastModified > lastModified) {
    lastModified = file.lastModified
    underlying = buildCredentials
  }

  override def refresh(): Unit = {
    reloadIfNeeded()
    underlying.refresh()
  }
  
  override def getRequestMetadata(uri: URI) = underlying.getRequestMetadata(uri)

 ...
}

A file based credentials is not always the most secure approach based on the environment and this plugins doesn't allow to use other approaches.

@abdolence
Copy link
Owner

I'm well aware that Credentials hierarchy exists and used by Google themselves to implement different kind of implementation. In fact Workspace Identity Federation and SA impersonalisation used quite often in CI pipelines with this plugin, and the approach where you trying to implement all those combination yourself and reading from file only I can't see as optimal.
Let me look into the goal itself, I'm pretty sure this can be implemented in the way which wouldn't require to extract Credentials to the settings level.

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

Successfully merging this pull request may close these issues.

2 participants