Deprecate the OAuth header in Nomulus tool (#2160)

Unless an --oauth flag is used, the nomulus tool will only send the OIDC
header. The server still accepts both headers and the user should use
`create_user` command to create an admin User (with the --oauth flag on), which
will then allow one to use the nomulus tool without the --oauth flag.

The --oauth flag and the server's ability to support OAuth-based
authentication will be removed soon. Users are urged to create the User
object in time to avoid service interruption.

TESTED=verified on alpha.
This commit is contained in:
Lai Jiang 2023-10-02 15:50:30 -04:00 committed by GitHub
parent cf43de7755
commit 1eed9c82dc
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 37 additions and 13 deletions

View file

@ -43,34 +43,41 @@ final class RegistryCli implements CommandRunner {
// The environment parameter is parsed twice: once here, and once with {@link // The environment parameter is parsed twice: once here, and once with {@link
// RegistryToolEnvironment#parseFromArgs} in the {@link RegistryTool#main} function. // RegistryToolEnvironment#parseFromArgs} in the {@link RegistryTool#main} function.
// //
// The flag names must be in sync between the two, and also - this is ugly and we should feel bad. // The flag names must be in sync between the two, and also - this is ugly, and we should feel
// bad.
@Parameter( @Parameter(
names = {"-e", "--environment"}, names = {"-e", "--environment"},
description = "Sets the default environment to run the command.") description = "Sets the default environment to run the command.")
private RegistryToolEnvironment environment = RegistryToolEnvironment.PRODUCTION; private RegistryToolEnvironment environment = RegistryToolEnvironment.PRODUCTION;
@Parameter(
names = "--oauth",
description =
"Turn on OAuth-based authentication, the usage of which is to be deprecated. Use"
+ " `create_user` to create an Admin user that allows for OIDC-based authentication.")
private boolean oAuth = false;
@Parameter( @Parameter(
names = {"-c", "--commands"}, names = {"-c", "--commands"},
description = "Returns all command names.") description = "Returns all command names.")
private boolean showAllCommands; private boolean showAllCommands;
@Parameter( @Parameter(
names = {"--credential"}, names = "--credential",
description = description =
"Name of a JSON file containing credential information used by the tool. " "Name of a JSON file containing credential information used by the tool. "
+ "If not set, credentials saved by running `nomulus login' will be used.") + "If not set, credentials saved by running `nomulus login' will be used.")
private String credentialJson = null; private String credentialJson = null;
@Parameter( @Parameter(
names = {"--sql_access_info"}, names = "--sql_access_info",
description = description =
"Name of a file containing space-separated SQL access info used when deploying " "Name of a file containing space-separated SQL access info used when deploying "
+ "Beam pipelines") + "Beam pipelines")
private String sqlAccessInfoFile = null; private String sqlAccessInfoFile = null;
// Do not make this final - compile-time constant inlining may interfere with JCommander. // Do not make this final - compile-time constant inlining may interfere with JCommander.
@ParametersDelegate @ParametersDelegate private LoggingParameters loggingParams = new LoggingParameters();
private LoggingParameters loggingParams = new LoggingParameters();
RegistryToolComponent component; RegistryToolComponent component;
@ -105,8 +112,8 @@ final class RegistryCli implements CommandRunner {
jcommander.setProgramName(programName); jcommander.setProgramName(programName);
// Create all command instances. It would be preferrable to do this in the constructor, but // Create all command instances. It would be preferrable to do this in the constructor, but
// JCommander mutates the command instances and doesn't reset them so we have to do it for every // JCommander mutates the command instances and doesn't reset them, so we have to do it for
// run. // every run.
try { try {
for (Map.Entry<String, ? extends Class<? extends Command>> entry : commands.entrySet()) { for (Map.Entry<String, ? extends Class<? extends Command>> entry : commands.entrySet()) {
Command command = entry.getValue().getDeclaredConstructor().newInstance(); Command command = entry.getValue().getDeclaredConstructor().newInstance();
@ -161,6 +168,7 @@ final class RegistryCli implements CommandRunner {
DaggerRegistryToolComponent.builder() DaggerRegistryToolComponent.builder()
.credentialFilePath(credentialJson) .credentialFilePath(credentialJson)
.sqlAccessInfoFile(sqlAccessInfoFile) .sqlAccessInfoFile(sqlAccessInfoFile)
.addOAuthHeader(oAuth)
.build(); .build();
// JCommander stores sub-commands as nested JCommander objects containing a list of user objects // JCommander stores sub-commands as nested JCommander objects containing a list of user objects

View file

@ -123,6 +123,7 @@ interface RegistryToolComponent {
void inject(GetDomainCommand command); void inject(GetDomainCommand command);
void inject(GetHostCommand command); void inject(GetHostCommand command);
void inject(GetKeyringSecretCommand command); void inject(GetKeyringSecretCommand command);
void inject(GetSqlCredentialCommand command); void inject(GetSqlCredentialCommand command);
@ -190,6 +191,9 @@ interface RegistryToolComponent {
@BindsInstance @BindsInstance
Builder sqlAccessInfoFile(@Nullable @Config("sqlAccessInfoFile") String sqlAccessInfoFile); Builder sqlAccessInfoFile(@Nullable @Config("sqlAccessInfoFile") String sqlAccessInfoFile);
@BindsInstance
Builder addOAuthHeader(@Config("addOauthHeader") boolean addOauthHeader);
RegistryToolComponent build(); RegistryToolComponent build();
} }
} }

View file

@ -42,7 +42,8 @@ final class RequestFactoryModule {
@Provides @Provides
static HttpRequestFactory provideHttpRequestFactory( static HttpRequestFactory provideHttpRequestFactory(
@ApplicationDefaultCredential GoogleCredentialsBundle credentialsBundle, @ApplicationDefaultCredential GoogleCredentialsBundle credentialsBundle,
@Config("oauthClientId") String oauthClientId) { @Config("oauthClientId") String oauthClientId,
@Config("addOauthHeader") boolean addOauthHeader) {
if (RegistryConfig.areServersLocal()) { if (RegistryConfig.areServersLocal()) {
return new NetHttpTransport() return new NetHttpTransport()
.createRequestFactory( .createRequestFactory(
@ -54,8 +55,10 @@ final class RequestFactoryModule {
return new NetHttpTransport() return new NetHttpTransport()
.createRequestFactory( .createRequestFactory(
request -> { request -> {
if (addOauthHeader) {
// Use the standard credential initializer to set the Authorization header // Use the standard credential initializer to set the Authorization header
credentialsBundle.getHttpRequestInitializer().initialize(request); credentialsBundle.getHttpRequestInitializer().initialize(request);
}
// Set OIDC token as the alternative bearer token. // Set OIDC token as the alternative bearer token.
request request
.getHeaders() .getHeaders()

View file

@ -64,7 +64,7 @@ public class RequestFactoryModuleTest {
RegistryConfig.CONFIG_SETTINGS.get().gcpProject.isLocal = true; RegistryConfig.CONFIG_SETTINGS.get().gcpProject.isLocal = true;
try { try {
HttpRequestFactory factory = HttpRequestFactory factory =
RequestFactoryModule.provideHttpRequestFactory(credentialsBundle, "client-id"); RequestFactoryModule.provideHttpRequestFactory(credentialsBundle, "client-id", false);
HttpRequestInitializer initializer = factory.getInitializer(); HttpRequestInitializer initializer = factory.getInitializer();
assertThat(initializer).isNotNull(); assertThat(initializer).isNotNull();
HttpRequest request = factory.buildGetRequest(new GenericUrl("http://localhost")); HttpRequest request = factory.buildGetRequest(new GenericUrl("http://localhost"));
@ -97,14 +97,23 @@ public class RequestFactoryModuleTest {
boolean origIsLocal = RegistryConfig.CONFIG_SETTINGS.get().gcpProject.isLocal; boolean origIsLocal = RegistryConfig.CONFIG_SETTINGS.get().gcpProject.isLocal;
RegistryConfig.CONFIG_SETTINGS.get().gcpProject.isLocal = false; RegistryConfig.CONFIG_SETTINGS.get().gcpProject.isLocal = false;
try { try {
// With OAuth header.
HttpRequestFactory factory = HttpRequestFactory factory =
RequestFactoryModule.provideHttpRequestFactory(credentialsBundle, "clientId"); RequestFactoryModule.provideHttpRequestFactory(credentialsBundle, "clientId", true);
HttpRequest request = factory.buildGetRequest(new GenericUrl("http://localhost")); HttpRequest request = factory.buildGetRequest(new GenericUrl("http://localhost"));
assertThat(request.getHeaders().get("Proxy-Authorization")).isEqualTo("Bearer oidc.token"); assertThat(request.getHeaders().get("Proxy-Authorization")).isEqualTo("Bearer oidc.token");
assertThat(request.getConnectTimeout()).isEqualTo(REQUEST_TIMEOUT_MS); assertThat(request.getConnectTimeout()).isEqualTo(REQUEST_TIMEOUT_MS);
assertThat(request.getReadTimeout()).isEqualTo(REQUEST_TIMEOUT_MS); assertThat(request.getReadTimeout()).isEqualTo(REQUEST_TIMEOUT_MS);
verify(httpRequestInitializer).initialize(request); verify(httpRequestInitializer).initialize(request);
verifyNoMoreInteractions(httpRequestInitializer); verifyNoMoreInteractions(httpRequestInitializer);
// No OAuth header.
factory =
RequestFactoryModule.provideHttpRequestFactory(credentialsBundle, "clientId", false);
request = factory.buildGetRequest(new GenericUrl("http://localhost"));
assertThat(request.getHeaders().get("Proxy-Authorization")).isEqualTo("Bearer oidc.token");
assertThat(request.getConnectTimeout()).isEqualTo(REQUEST_TIMEOUT_MS);
assertThat(request.getReadTimeout()).isEqualTo(REQUEST_TIMEOUT_MS);
verifyNoMoreInteractions(httpRequestInitializer);
} finally { } finally {
RegistryConfig.CONFIG_SETTINGS.get().gcpProject.isLocal = origIsLocal; RegistryConfig.CONFIG_SETTINGS.get().gcpProject.isLocal = origIsLocal;
} }