Fix tokens auth type determinatin (#35062)oidc-1.2.7a

6 files changed, 219 insertions, 42 deletions

diff --git a/ydb/mvp/core/mvp_startup_options.cpp b/ydb/mvp/core/mvp_startup_options.cpp
index 2c776c90722..d935548e340 100644
--- a/ydb/mvp/core/mvp_startup_options.cpp
+++ b/ydb/mvp/core/mvp_startup_options.cpp
@@ -53,6 +53,15 @@ void MergeRepeatedByName(google::protobuf::RepeatedPtrField<TMessage>* dst,
     MergeRepeatedByName(dst, src, [](TMessage*) {});
 }
 
+void EnsureAccessServiceTypeMatch(const std::optional<NMvp::EAccessServiceType>& left,
+                                  const std::optional<NMvp::EAccessServiceType>& right,
+                                  TStringBuf message)
+{
+    if (left && right && *left != *right) {
+        ythrow yexception() << CONFIG_ERROR_PREFIX << message;
+    }
+}
+
 } // namespace
 
 TMvpStartupOptions TMvpStartupOptions::Build(int argc, const char* argv[]) {
@@ -61,8 +70,12 @@ TMvpStartupOptions TMvpStartupOptions::Build(int argc, const char* argv[]) {
     NLastGetopt::TOptsParseResult parsedArgs = startupOptions.ParseArgs(argc, argv);
     startupOptions.LoadConfig(parsedArgs);
     startupOptions.SetPorts();
-    startupOptions.LoadTokens();
-    startupOptions.OverrideTokensConfig();
+    startupOptions.LoadTokensFromTokenFile();
+    startupOptions.OverrideTokensFromConfig();
+    startupOptions.MergeAccessServiceType();
+    startupOptions.MigrateJwtInfoToOAuth2Exchange();
+    startupOptions.ValidateOAuth2ExchangeTokenNames(startupOptions.Tokens.GetOAuth2Exchange(), "token file config");
+    startupOptions.ValidateOAuth2ExchangeTokenEndpointScheme(startupOptions.Tokens.GetOAuth2Exchange(), "token file config");
     startupOptions.ValidateTokensConfig();
     startupOptions.LoadCertificates();
 
@@ -115,7 +128,7 @@ void TMvpStartupOptions::LoadConfig(const NLastGetopt::TOptsParseResult& parsedA
 
 void TMvpStartupOptions::TryGetStartupOptionsFromConfig(const NLastGetopt::TOptsParseResult& parsedArgs, const NMvp::TGenericConfig& generic) {
     if (generic.HasAccessServiceType()) {
-        AccessServiceType = generic.GetAccessServiceType();
+        AccessServiceTypeFromConfig = generic.GetAccessServiceType();
     }
 
     if (generic.HasLogging()
@@ -136,8 +149,17 @@ void TMvpStartupOptions::TryGetStartupOptionsFromConfig(const NLastGetopt::TOpts
         }
 
         if (auth.HasTokens()) {
-            ValidateTokensOverrideConfig(auth.GetTokens());
-            TokensOverrideConfig = auth.GetTokens();
+            const auto& tokensFromConfig = auth.GetTokens();
+            if (tokensFromConfig.HasAccessServiceType()) {
+                EnsureAccessServiceTypeMatch(
+                    AccessServiceTypeFromConfig,
+                    std::make_optional(tokensFromConfig.GetAccessServiceType()),
+                    "auth.tokens.access_service_type must match access_service_type");
+                AccessServiceTypeFromConfig = tokensFromConfig.GetAccessServiceType();
+            }
+
+            ValidateTokensFromConfig(tokensFromConfig);
+            TokensFromConfig = tokensFromConfig;
         }
     }
 
@@ -183,34 +205,41 @@ TString TMvpStartupOptions::AddSchemeToUserToken(const TString& token, const TSt
     return scheme + " " + token;
 }
 
-void TMvpStartupOptions::LoadTokens() {
-    Tokens.SetAccessServiceType(AccessServiceType);
-
-    if (YdbTokenFile.empty()) {
-        return;
-    }
-
-    if (google::protobuf::TextFormat::ParseFromString(TUnbufferedFileInput(YdbTokenFile).ReadAll(), &Tokens)) {
-        MigrateJwtInfoToOAuth2Exchange();
-        ValidateOAuth2ExchangeTokenNames(Tokens.GetOAuth2Exchange(), "token file config");
-        ValidateOAuth2ExchangeTokenEndpointScheme(Tokens.GetOAuth2Exchange(), "token file config");
-        if (Tokens.HasStaffApiUserTokenInfo()) {
-            UserToken = Tokens.GetStaffApiUserTokenInfo().GetToken();
-        } else if (Tokens.HasStaffApiUserToken()) {
-            UserToken = Tokens.GetStaffApiUserToken();
+void TMvpStartupOptions::LoadTokensFromTokenFile() {
+    if (!YdbTokenFile.empty()) {
+        if (google::protobuf::TextFormat::ParseFromString(TUnbufferedFileInput(YdbTokenFile).ReadAll(), &Tokens)) {
+            if (Tokens.HasAccessServiceType()) {
+                AccessServiceTypeFromTokenFile = Tokens.GetAccessServiceType();
+            }
+            if (Tokens.HasStaffApiUserTokenInfo()) {
+                UserToken = Tokens.GetStaffApiUserTokenInfo().GetToken();
+            } else if (Tokens.HasStaffApiUserToken()) {
+                UserToken = Tokens.GetStaffApiUserToken();
+            }
+            UserToken = AddSchemeToUserToken(UserToken, "OAuth");
+        } else {
+            ythrow yexception() << CONFIG_ERROR_PREFIX << "Invalid ydb token file format";
         }
-        UserToken = AddSchemeToUserToken(UserToken, "OAuth");
-    } else {
-        ythrow yexception() << CONFIG_ERROR_PREFIX << "Invalid ydb token file format";
     }
 }
 
-void TMvpStartupOptions::OverrideTokensConfig() {
-    if (!TokensOverrideConfig.has_value()) {
+void TMvpStartupOptions::MergeAccessServiceType() {
+    EnsureAccessServiceTypeMatch(AccessServiceTypeFromConfig,
+                                 AccessServiceTypeFromTokenFile,
+                                 "token file access_service_type must match access_service_type");
+
+    AccessServiceType = AccessServiceTypeFromConfig.value_or(
+        AccessServiceTypeFromTokenFile.value_or(NMvp::yandex_v2));
+
+    Tokens.SetAccessServiceType(AccessServiceType);
+}
+
+void TMvpStartupOptions::OverrideTokensFromConfig() {
+    if (!TokensFromConfig.has_value()) {
         return;
     }
 
-    const auto& override = *TokensOverrideConfig;
+    const auto& override = *TokensFromConfig;
 
     if (override.HasStaffApiUserToken()) {
         Tokens.SetStaffApiUserToken(override.GetStaffApiUserToken());
@@ -218,12 +247,6 @@ void TMvpStartupOptions::OverrideTokensConfig() {
     if (override.HasStaffApiUserTokenInfo()) {
         Tokens.MutableStaffApiUserTokenInfo()->MergeFrom(override.GetStaffApiUserTokenInfo());
     }
-    if (override.HasAccessServiceType()) {
-        if (override.GetAccessServiceType() != AccessServiceType) {
-            ythrow yexception() << CONFIG_ERROR_PREFIX
-                                << "auth.tokens.access_service_type must match access_service_type";
-        }
-    }
 
     MergeRepeatedByName(Tokens.MutableJwtInfo(), override.GetJwtInfo());
     MergeRepeatedByName(Tokens.MutableOAuthInfo(), override.GetOAuthInfo());
@@ -231,9 +254,6 @@ void TMvpStartupOptions::OverrideTokensConfig() {
     MergeRepeatedByName(Tokens.MutableMetadataTokenInfo(), override.GetMetadataTokenInfo());
     MergeRepeatedByName(Tokens.MutableStaticCredentialsInfo(), override.GetStaticCredentialsInfo());
     MergeRepeatedByName(Tokens.MutableOAuth2Exchange(), override.GetOAuth2Exchange());
-    MigrateJwtInfoToOAuth2Exchange();
-    ValidateOAuth2ExchangeTokenNames(Tokens.GetOAuth2Exchange(), "merged oauth2_exchange token config");
-    Tokens.SetAccessServiceType(AccessServiceType);
 }
 
 void TMvpStartupOptions::LoadCertificates() {
diff --git a/ydb/mvp/core/mvp_startup_options.h b/ydb/mvp/core/mvp_startup_options.h
index 89796c6a405..ab8da65d865 100644
--- a/ydb/mvp/core/mvp_startup_options.h
+++ b/ydb/mvp/core/mvp_startup_options.h
@@ -23,9 +23,11 @@ private:
     TString YdbTokenFile;
     TString CaCertificateFile;
     TString SslCertificateFile;
+    std::optional<NMvp::EAccessServiceType> AccessServiceTypeFromConfig;
+    std::optional<NMvp::EAccessServiceType> AccessServiceTypeFromTokenFile;
 
 public:
-    std::optional<NMvp::TTokensConfig> TokensOverrideConfig;
+    std::optional<NMvp::TTokensConfig> TokensFromConfig;
 
     bool LogToStderr = false;
     bool Mlock = false;
@@ -48,8 +50,9 @@ private:
     void TryGetStartupOptionsFromConfig(const NLastGetopt::TOptsParseResult& parsedArgs, const NMvp::TGenericConfig& generic);
     void SetPorts();
     TString AddSchemeToUserToken(const TString& token, const TString& scheme);
+    void MergeAccessServiceType();
     void MigrateJwtInfoToOAuth2Exchange();
-    void ValidateTokensOverrideConfig(const NMvp::TTokensConfig& tokensOverride);
+    void ValidateTokensFromConfig(const NMvp::TTokensConfig& tokensOverride);
     void ValidateOAuth2ExchangeTokenEndpointScheme(const google::protobuf::RepeatedPtrField<NMvp::TOAuth2Exchange>& oauth2Exchange,
                                                    const TString& configSource);
     void ValidateOAuth2ExchangeTokenNames(const google::protobuf::RepeatedPtrField<NMvp::TOAuth2Exchange>& oauth2Exchange,
@@ -57,9 +60,9 @@ private:
     void ValidateOAuth2CredentialsFields(const NMvp::TOAuth2Exchange::TCredentials& creds,
                                          const TString& credsRole,
                                          const TString& tokenName);
-    void OverrideTokensConfig();
+    void OverrideTokensFromConfig();
     void ValidateTokensConfig();
-    void LoadTokens();
+    void LoadTokensFromTokenFile();
     void LoadCertificates();
 };
 
diff --git a/ydb/mvp/core/mvp_startup_options_migration_ut.cpp b/ydb/mvp/core/mvp_startup_options_migration_ut.cpp
index 322118f122f..a6154c4e058 100644
--- a/ydb/mvp/core/mvp_startup_options_migration_ut.cpp
+++ b/ydb/mvp/core/mvp_startup_options_migration_ut.cpp
@@ -6,6 +6,110 @@
 using namespace NMVP;
 
 Y_UNIT_TEST_SUITE(TMvpStartupOptionsMigration) {
+    Y_UNIT_TEST(TokenFileAccessServiceTypeUsedWhenMissingInConfig) {
+        TTempFileHandle tmpToken = MakeTestFile(R"pb(
+AccessServiceType: nebius_v1
+JwtInfo {
+  Name: "legacy-jwt"
+  Endpoint: "grpcs://token.endpoint:443"
+  AccountId: "service-account-id"
+  KeyId: "key-id"
+  PrivateKey: "private-key"
+}
+)pb", "mvp_legacy_jwt_with_access_service_type", ".pb.txt");
+
+        TString yaml = TStringBuilder() << R"(
+generic:
+  auth:
+    token_file: )" << tmpToken.Name() << "\n";
+        TTempFileHandle tmpYaml = MakeTestFile(yaml, "mvp_startup_token_file_access_service_type", ".yaml");
+
+        const char* argv[] = {"mvp_test", "--config", tmpYaml.Name().c_str()};
+        TMvpStartupOptions opts = MakeOpts(argv);
+
+        UNIT_ASSERT(opts.Tokens.GetAccessServiceType() == NMvp::nebius_v1);
+        UNIT_ASSERT(opts.AccessServiceType == NMvp::nebius_v1);
+    }
+
+    Y_UNIT_TEST(TokenFileAndConfigAccessServiceTypeMismatchThrows) {
+        TTempFileHandle tmpToken = MakeTestFile(R"pb(
+AccessServiceType: yandex_v2
+JwtInfo {
+  Name: "legacy-jwt"
+  Endpoint: "grpcs://token.endpoint:443"
+  AccountId: "service-account-id"
+  KeyId: "key-id"
+  PrivateKey: "private-key"
+}
+)pb", "mvp_legacy_jwt_access_service_type_mismatch", ".pb.txt");
+
+        TString yaml = TStringBuilder() << R"(
+generic:
+  access_service_type: "nebius_v1"
+  auth:
+    token_file: )" << tmpToken.Name() << "\n";
+        TTempFileHandle tmpYaml = MakeTestFile(yaml, "mvp_startup_access_service_type_mismatch", ".yaml");
+
+        const char* argv[] = {"mvp_test", "--config", tmpYaml.Name().c_str()};
+        UNIT_ASSERT_EXCEPTION_CONTAINS(MakeOpts(argv), yexception, "token file access_service_type must match access_service_type");
+    }
+
+    Y_UNIT_TEST(TokenFileAndAuthTokensAccessServiceTypeMismatchThrows) {
+        TTempFileHandle tmpToken = MakeTestFile(R"pb(
+AccessServiceType: yandex_v2
+JwtInfo {
+  Name: "legacy-jwt"
+  Endpoint: "grpcs://token.endpoint:443"
+  AccountId: "service-account-id"
+  KeyId: "key-id"
+  PrivateKey: "private-key"
+}
+)pb", "mvp_legacy_jwt_auth_tokens_access_service_type_mismatch", ".pb.txt");
+
+        TString yaml = TStringBuilder() << R"(
+generic:
+  auth:
+    token_file: )" << tmpToken.Name() << R"(
+    tokens:
+      access_service_type: "nebius_v1"
+)";
+        TTempFileHandle tmpYaml = MakeTestFile(yaml, "mvp_startup_auth_tokens_access_service_type_mismatch", ".yaml");
+
+        const char* argv[] = {"mvp_test", "--config", tmpYaml.Name().c_str()};
+        UNIT_ASSERT_EXCEPTION_CONTAINS(MakeOpts(argv), yexception, "token file access_service_type must match access_service_type");
+    }
+
+    Y_UNIT_TEST(TokenFileWithoutAccessServiceTypeUsesGenericValue) {
+        TTempFileHandle tmpToken = MakeTestFile(R"pb(
+JwtInfo {
+  Name: "legacy-jwt"
+  Endpoint: "grpcs://token.endpoint:443"
+  AccountId: "service-account-id"
+  KeyId: "key-id"
+  PrivateKey: "private-key"
+  Audience: "legacy-audience"
+}
+)pb", "mvp_legacy_jwt_no_access_service_type", ".pb.txt");
+
+        TString yaml = TStringBuilder() << R"(
+generic:
+  access_service_type: "nebius_v1"
+  auth:
+    token_file: )" << tmpToken.Name() << "\n";
+        TTempFileHandle tmpYaml = MakeTestFile(yaml, "mvp_startup_nebius_jwt_migration_no_access_service_type", ".yaml");
+
+        const char* argv[] = {"mvp_test", "--config", tmpYaml.Name().c_str()};
+        TMvpStartupOptions opts = MakeOpts(argv);
+
+        UNIT_ASSERT(opts.Tokens.GetAccessServiceType() == NMvp::nebius_v1);
+        UNIT_ASSERT_VALUES_EQUAL(opts.Tokens.JwtInfoSize(), 0);
+        UNIT_ASSERT_VALUES_EQUAL(opts.Tokens.OAuth2ExchangeSize(), 1);
+        const auto& tokenExchange = opts.Tokens.GetOAuth2Exchange(0);
+        UNIT_ASSERT(tokenExchange.HasSubjectCredentials());
+        UNIT_ASSERT_VALUES_EQUAL(tokenExchange.GetSubjectCredentials().GetAlg(), "RS256");
+        UNIT_ASSERT_VALUES_EQUAL(tokenExchange.GetSubjectCredentials().GetSub(), "service-account-id");
+    }
+
     Y_UNIT_TEST(NebiusTokenFileMigratesJwtInfoToOauth2Exchange) {
         TTempFileHandle tmpToken = MakeTestFile(R"pb(
 AccessServiceType: nebius_v1
diff --git a/ydb/mvp/core/mvp_startup_options_validation.cpp b/ydb/mvp/core/mvp_startup_options_validation.cpp
index 74a74b02dd1..245bc4aebf8 100644
--- a/ydb/mvp/core/mvp_startup_options_validation.cpp
+++ b/ydb/mvp/core/mvp_startup_options_validation.cpp
@@ -42,8 +42,8 @@ void TMvpStartupOptions::ValidateOAuth2ExchangeTokenEndpointScheme(
     }
 }
 
-void TMvpStartupOptions::ValidateTokensOverrideConfig(const NMvp::TTokensConfig& tokensOverride) {
-    if (AccessServiceType != NMvp::nebius_v1) {
+void TMvpStartupOptions::ValidateTokensFromConfig(const NMvp::TTokensConfig& tokensOverride) {
+    if (AccessServiceTypeFromConfig && *AccessServiceTypeFromConfig != NMvp::nebius_v1) {
         ythrow yexception() << CONFIG_ERROR_PREFIX << "auth.tokens overrides are only supported for Nebius access service type";
     }
     ValidateOAuth2ExchangeTokenNames(tokensOverride.GetOAuth2Exchange(), "'auth.tokens.oauth2_exchange'");
diff --git a/ydb/mvp/oidc_proxy/mvp.cpp b/ydb/mvp/oidc_proxy/mvp.cpp
index 91e1682f4c5..991015f6c3f 100644
--- a/ydb/mvp/oidc_proxy/mvp.cpp
+++ b/ydb/mvp/oidc_proxy/mvp.cpp
@@ -244,6 +244,10 @@ THolder<NActors::TActorSystemSetup> TMVP::BuildActorSystemSetup() {
         ythrow yexception() << NMVP::CONFIG_ERROR_PREFIX
                             << "SessionServiceTokenName must be specified in oidc config.";
     }
+    if (OpenIdConnectSettings.SecretName.empty()) {
+        ythrow yexception() << NMVP::CONFIG_ERROR_PREFIX
+                            << "SecretName must be specified in oidc config.";
+    }
     OpenIdConnectSettings.InitRequestTimeoutsByPath();
 
     if (StartupOptions.Mlock) {
@@ -269,7 +273,7 @@ THolder<NActors::TActorSystemSetup> TMVP::BuildActorSystemSetup() {
             break;
         }
     }
-    if (!OpenIdConnectSettings.SecretName.empty() && !clientSecretFound) {
+    if (!clientSecretFound) {
         ythrow yexception() << NMVP::CONFIG_ERROR_PREFIX
                             << "oidc.secret_name '" << OpenIdConnectSettings.SecretName
                             << "' was not found in auth token config secret_info.";
diff --git a/ydb/mvp/oidc_proxy/mvp_config_validation_ut.cpp b/ydb/mvp/oidc_proxy/mvp_config_validation_ut.cpp
index 1fc1eb3cafb..2b67178ab8d 100644
--- a/ydb/mvp/oidc_proxy/mvp_config_validation_ut.cpp
+++ b/ydb/mvp/oidc_proxy/mvp_config_validation_ut.cpp
@@ -50,4 +50,50 @@ oidc:
         const char* argv[] = {"mvp_test", "--config", tmpYaml.Name().c_str()};
         UNIT_ASSERT_EXCEPTION_CONTAINS(TMVP(3, argv), yexception, "was not found in auth token config secret_info");
     }
+
+    Y_UNIT_TEST(SecretNameRequired) {
+        TTempFileHandle tmpYaml = MakeTestFile(R"(
+generic:
+  access_service_type: "nebius_v1"
+oidc:
+  client_id: "test-client"
+  session_service_endpoint: "localhost:8655"
+  session_service_token_name: "service-account-jwt"
+  authorization_server_address: "http://auth.test.net"
+)", "mvp_oidc_proxy_missing_secret_name", ".yaml");
+
+        const char* argv[] = {"mvp_test", "--config", tmpYaml.Name().c_str()};
+        UNIT_ASSERT_EXCEPTION_CONTAINS(TMVP(3, argv), yexception, "SecretName must be specified");
+    }
+
+    Y_UNIT_TEST(SecretInfoValueMustNotBeEmpty) {
+        TTempFileHandle tmpTokenFile = MakeTestFile(R"pb(
+OAuthInfo {
+  Name: "service-account-jwt"
+  Endpoint: "grpc://localhost:8655"
+  Token: "oauth-token"
+}
+SecretInfo {
+  Name: "secret-name"
+  Secret: ""
+}
+)pb", "mvp_oidc_proxy_secret_info_empty", ".pb.txt");
+
+        TString yaml = TStringBuilder() << R"(
+generic:
+  access_service_type: "yandex_v2"
+  auth:
+    token_file: )" << tmpTokenFile.Name() << R"(
+oidc:
+  client_id: "test-client"
+  secret_name: "secret-name"
+  session_service_endpoint: "localhost:8655"
+  session_service_token_name: "service-account-jwt"
+  authorization_server_address: "http://auth.test.net"
+)";
+        TTempFileHandle tmpYaml = MakeTestFile(yaml, "mvp_oidc_proxy_empty_secret_info", ".yaml");
+
+        const char* argv[] = {"mvp_test", "--config", tmpYaml.Name().c_str()};
+        UNIT_ASSERT_EXCEPTION_CONTAINS(TMVP(3, argv), yexception, "has empty value in auth token config secret_info");
+    }
 }