Skip to content

Commit

Permalink
New application configuration: hoist.sensitiveParamTerms (#407)
Browse files Browse the repository at this point in the history
  • Loading branch information
lbwexler authored Sep 27, 2024
1 parent b424f50 commit ff6bb12
Show file tree
Hide file tree
Showing 5 changed files with 32 additions and 18 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@
This replaces the static map `BaseService.clusterConfigs`.
* Misc. improvements to logging and performance of `Cache` and `Timer`.

### 🎁 New Features
* New configuration property `hoist.sensitiveParamTerms` allows customization of environment
variables to be obscured in the admin client.

## 22.0.0 - 2024-09-18

### 💥 Breaking Changes (upgrade difficulty: 🟢 LOW)
Expand Down
4 changes: 0 additions & 4 deletions grails-app/domain/io/xh/hoist/config/AppConfig.groovy
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import org.jasypt.util.text.BasicTextEncryptor
import org.jasypt.util.text.TextEncryptor

import static grails.async.Promises.task
import static io.xh.hoist.util.Utils.isSensitiveParamName

class AppConfig implements JSONFormat, LogSupport {

Expand Down Expand Up @@ -121,9 +120,6 @@ class AppConfig implements JSONFormat, LogSupport {
// so we don't assume it can be parsed into the required type. Log failures on trace for
// minimal visibility, but otherwise act as if the override value is not set.
try {
if (isSensitiveParamName(name) && valueType != 'pwd') {
throw new RuntimeException("Refusing to return potentially sensitive override value from plain-text config - must be of type 'pwd'")
}
return parseValue(overrideValue, opts)
} catch (Throwable e) {
logTrace("InstanceConfig override found for '$name' but cannot be parsed - override will be ignored", e.message)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@

package io.xh.hoist.websocket

import grails.async.Promises
import grails.core.GrailsApplication
import grails.events.EventPublisher
import groovy.transform.CompileStatic
import io.xh.hoist.BaseService
Expand All @@ -23,6 +21,7 @@ import java.util.concurrent.ConcurrentHashMap

import static grails.async.Promises.task
import static grails.async.Promises.waitAll
import static io.xh.hoist.util.Utils.grailsConfig


/**
Expand Down Expand Up @@ -50,7 +49,6 @@ import static grails.async.Promises.waitAll
@CompileStatic
class WebSocketService extends BaseService implements EventPublisher {

GrailsApplication grailsApplication
IdentityService identityService

static final String HEARTBEAT_TOPIC = 'xhHeartbeat'
Expand All @@ -62,7 +60,7 @@ class WebSocketService extends BaseService implements EventPublisher {
private Map<WebSocketSession, HoistWebSocketChannel> _channels = new ConcurrentHashMap<>()

boolean isEnabled() {
return grailsApplication.config.getProperty('hoist.enableWebSockets', Boolean)
grailsConfig.getProperty('hoist.enableWebSockets', Boolean)
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,10 @@ class ApplicationConfig {
static void defaultConfig(Script script) {
withDelegate(script) {

// Read by WebSocketService to determine if WS support should generally be enabled.
hoist {
// Read by WebSocketService to determine if WS support should generally be enabled.
enableWebSockets = true
sensitiveParamTerms = ['password', 'passwrd', 'pwd', 'secret', 'tkn', 'token']
}

spring {
Expand Down
33 changes: 24 additions & 9 deletions src/main/groovy/io/xh/hoist/util/Utils.groovy
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import com.esotericsoftware.kryo.Kryo
import com.esotericsoftware.kryo.io.Input
import com.esotericsoftware.kryo.io.Output
import com.esotericsoftware.kryo.serializers.JavaSerializer
import grails.config.Config
import grails.util.Environment
import grails.util.Holders
import grails.util.Metadata
Expand Down Expand Up @@ -151,21 +152,24 @@ class Utils {
//------------------
// Other Singletons
//------------------
/** Get the grails application context. */
static ApplicationContext getAppContext() {
return Holders.applicationContext
}

/** Get the grails application config. */
static Config getGrailsConfig() {
Holders.grailsApplication.config
}

/** Primary JDBC datasource, default backing DB for app Domain objects. */
static DataSource getDataSource() {
return (DataSource) appContext.dataSource
}

/** Primary dataSource configuration. */
static Map getDataSourceConfig() {
Holders.grailsApplication
.config
.getProperty('dataSource', Map.class)
.collectEntries { it }
grailsConfig.getProperty('dataSource', Map.class).collectEntries { it }
}

static ExceptionHandler getExceptionHandler() {
Expand All @@ -181,13 +185,14 @@ class Utils {
}

/**
* True if the given parameter name is likely sensitive and should not be serialized if not
* required. Used for built-in admin functionality accessed by trusted users only.
* NOT intended to be comprehensive or the last word on security!
* True if the given parameter name is likely sensitive and should not be serialized.
* To customize, set `hoist.sensitiveParamTerms` within your app's `application.groovy` to a
* list of terms that should trigger this behavior.
*
* See {@link io.xh.hoist.configuration.ApplicationConfig} for the default list.
*/
static boolean isSensitiveParamName(String name) {
def pattern = ~/(?i)(password|pwd|secret)$/
pattern.matcher(name).find()
sensitiveParams.any { name.containsIgnoreCase(it) }
}

/** String parsing for a boolean. */
Expand Down Expand Up @@ -220,6 +225,7 @@ class Utils {
c.call()
}


static testSerialization(Object obj, Class clazz, LogSupport logSupport, Map opts) {
Kryo kryo = new Kryo()
kryo.registrationRequired = false
Expand Down Expand Up @@ -251,4 +257,13 @@ class Utils {
"${endTime - startTime}ms"
)
}

private static terms = null
private static List<String> getSensitiveParams() {
if (terms == null) {
terms = grailsConfig.getProperty('hoist.sensitiveParamTerms', ArrayList.class) as List<String>
}
return terms
}

}

0 comments on commit ff6bb12

Please sign in to comment.