Skip to content

Commit 35a8906

Browse files
molotkov-andmolotkov-and
committed
small fixes
1 parent 50201b3 commit 35a8906

File tree

4 files changed

+96
-98
lines changed

4 files changed

+96
-98
lines changed

‎ydb/core/security/ticket_parser_impl.h‎

Lines changed: 94 additions & 95 deletions
Original file line numberDiff line numberDiff line change
@@ -388,8 +388,7 @@ class TTicketParserImpl : public TActorBootstrapped<TDerived> {
388388

389389
template <typename TTokenRecord>
390390
void AccessServiceAuthorize(const TString& key, TTokenRecord& record) const {
391-
for (const auto& [permissionName, permRecord] : record.Permissions) {
392-
// const TString& permission(perm);
391+
for (const auto& [permissionName, permissionRecord] : record.Permissions) {
393392
BLOG_TRACE("Ticket " << record.GetMaskedTicket() << " asking for AccessServiceAuthorization(" << permissionName << ")");
394393

395394
auto request = CreateAccessServiceRequest<TEvAccessServiceAuthorizeRequest>(key, record);
@@ -791,6 +790,19 @@ class TTicketParserImpl : public TActorBootstrapped<TDerived> {
791790
}
792791
}
793792

793+
template <typename TTokenRecord>
794+
void SetAccessServiceBulkAuthorizeError(const TString& key, TTokenRecord& record, const TString& errorMessage, bool isRetryableError) {
795+
for (auto& [permissionName, permissionRecord] : record.Permissions) {
796+
permissionRecord.Subject.clear();
797+
permissionRecord.Error = {.Message = errorMessage, .Retryable = isRetryableError};
798+
BLOG_TRACE("Ticket " << record.GetMaskedTicket()
799+
<< " permission " << permissionName
800+
<< " now has a " << (isRetryableError ? "retryable" : "permanent") << " error \"" << errorMessage << "\""
801+
<< " retryable: " << isRetryableError);
802+
}
803+
SetError(key, record, {.Message = errorMessage, .Retryable = isRetryableError});
804+
}
805+
794806
void Handle(NCloud::TEvAccessService::TEvBulkAuthorizeResponse::TPtr& ev) {
795807
auto getResourcePathIdForRequiredPermissions = [] (const ::yandex::cloud::priv::accessservice::v2::Resource& resource_path) -> TString {
796808
if (resource_path.type() == "resource-manager.folder") {
@@ -819,107 +831,94 @@ class TTicketParserImpl : public TActorBootstrapped<TDerived> {
819831
--record.ResponsesLeft;
820832
auto& examinedPermissions = record.Permissions;
821833
if (response->Status.Ok()) {
822-
const auto& subject = response->Response.subject();
823-
const TString subjectName = GetSubjectName(subject);
824-
const auto& subjectType = ConvertSubjectType(subject.type_case());
825-
for (auto& [permissionName, permissionRecord] : examinedPermissions) {
826-
permissionRecord.Subject = subjectName;
827-
permissionRecord.SubjectType = subjectType;
828-
permissionRecord.Error.clear();
829-
}
830-
size_t permissionDeniedCount = 0;
831-
bool hasRequiredPermissionFailed = false;
832-
std::vector<typename THashMap<TString, TPermissionRecord>::iterator> requiredPermissions;
833-
TString permissionDeniedError;
834-
const auto& results = response->Response.results();
835-
for (const auto& result : results.items()) {
836-
auto permissionDeniedIt = examinedPermissions.find(result.permission());
837-
if (permissionDeniedIt != examinedPermissions.end()) {
838-
permissionDeniedCount++;
839-
auto& permissionDeniedRecord = permissionDeniedIt->second;
840-
permissionDeniedRecord.Subject.clear();
841-
BLOG_TRACE("Ticket " << record.GetMaskedTicket() << " permission " << result.permission() << " access denied for subject \"" << subjectName << "\"");
842-
TStringBuilder errorMessage;
843-
if (permissionDeniedRecord.IsRequired()) {
844-
hasRequiredPermissionFailed = true;
845-
errorMessage << permissionDeniedIt->first << " for";
846-
for (const auto& resourcePath : result.resource_path()) {
847-
errorMessage << getResourcePathIdForRequiredPermissions(resourcePath);
848-
}
849-
errorMessage << " - ";
850-
requiredPermissions.push_back(permissionDeniedIt);
851-
}
852-
permissionDeniedError = result.permission_denied_error().message();;
853-
errorMessage << permissionDeniedError;
854-
permissionDeniedRecord.Error = {.Message = errorMessage, .Retryable = false};
855-
} else {
856-
BLOG_W("Received response for unknown permission " << result.permission() << " for ticket " << record.GetMaskedTicket());
834+
if (response->Response.has_unauthenticated_error()) {
835+
SetAccessServiceBulkAuthorizeError(key, record, response->Response.unauthenticated_error().message(), false);
836+
} else {
837+
const auto& subject = response->Response.subject();
838+
const TString subjectName = GetSubjectName(subject);
839+
const auto& subjectType = ConvertSubjectType(subject.type_case());
840+
for (auto& [permissionName, permissionRecord] : examinedPermissions) {
841+
permissionRecord.Subject = subjectName;
842+
permissionRecord.SubjectType = subjectType;
843+
permissionRecord.Error.clear();
857844
}
858-
}
859-
if (permissionDeniedCount < examinedPermissions.size() && !hasRequiredPermissionFailed) {
860-
record.TokenType = request->Request.has_api_key() ? TDerived::ETokenType::ApiKey : TDerived::ETokenType::AccessService;
861-
switch (subjectType) {
862-
case TPermissionRecord::TTypeCase::USER_ACCOUNT_TYPE:
863-
if (UserAccountService) {
864-
BLOG_TRACE("Ticket " << record.GetMaskedTicket()
865-
<< " asking for UserAccount(" << subjectName << ")");
866-
THolder<TEvAccessServiceGetUserAccountRequest> request = MakeHolder<TEvAccessServiceGetUserAccountRequest>(key);
867-
request->Token = record.Ticket;
868-
request->Request.set_user_account_id(subject.user_account().id());
869-
record.ResponsesLeft++;
870-
Send(UserAccountService, request.Release());
871-
return;
872-
}
873-
break;
874-
case TPermissionRecord::TTypeCase::SERVICE_ACCOUNT_TYPE:
875-
if (ServiceAccountService) {
876-
BLOG_TRACE("Ticket " << record.GetMaskedTicket()
877-
<< " asking for ServiceAccount(" << subjectName << ")");
878-
THolder<TEvAccessServiceGetServiceAccountRequest> request = MakeHolder<TEvAccessServiceGetServiceAccountRequest>(key);
879-
request->Token = record.Ticket;
880-
request->Request.set_service_account_id(subject.service_account().id());
881-
record.ResponsesLeft++;
882-
Send(ServiceAccountService, request.Release());
883-
return;
845+
size_t permissionDeniedCount = 0;
846+
bool hasRequiredPermissionFailed = false;
847+
std::vector<typename THashMap<TString, TPermissionRecord>::iterator> requiredPermissions;
848+
TString permissionDeniedError;
849+
const auto& results = response->Response.results();
850+
for (const auto& result : results.items()) {
851+
auto permissionDeniedIt = examinedPermissions.find(result.permission());
852+
if (permissionDeniedIt != examinedPermissions.end()) {
853+
permissionDeniedCount++;
854+
auto& permissionDeniedRecord = permissionDeniedIt->second;
855+
permissionDeniedRecord.Subject.clear();
856+
BLOG_TRACE("Ticket " << record.GetMaskedTicket() << " permission " << result.permission() << " access denied for subject \"" << subjectName << "\"");
857+
TStringBuilder errorMessage;
858+
if (permissionDeniedRecord.IsRequired()) {
859+
hasRequiredPermissionFailed = true;
860+
errorMessage << permissionDeniedIt->first << " for";
861+
for (const auto& resourcePath : result.resource_path()) {
862+
errorMessage << getResourcePathIdForRequiredPermissions(resourcePath);
863+
}
864+
errorMessage << " - ";
865+
requiredPermissions.push_back(permissionDeniedIt);
866+
}
867+
permissionDeniedError = result.permission_denied_error().message();;
868+
errorMessage << permissionDeniedError;
869+
permissionDeniedRecord.Error = {.Message = errorMessage, .Retryable = false};
870+
} else {
871+
BLOG_W("Received response for unknown permission " << result.permission() << " for ticket " << record.GetMaskedTicket());
884872
}
885-
break;
886-
default:
887-
break;
888873
}
889-
SetToken(request->Key, record, new NACLib::TUserToken(record.Ticket, subjectName, {}));
890-
} else {
891-
if (hasRequiredPermissionFailed) {
892-
TStringBuilder errorMessage;
893-
auto it = requiredPermissions.cbegin();
894-
errorMessage << (*it)->second.Error.Message;
895-
++it;
896-
for (; it != requiredPermissions.cend(); ++it) {
897-
errorMessage << ", " << (*it)->second.Error.Message;
874+
if (permissionDeniedCount < examinedPermissions.size() && !hasRequiredPermissionFailed) {
875+
record.TokenType = request->Request.has_api_key() ? TDerived::ETokenType::ApiKey : TDerived::ETokenType::AccessService;
876+
switch (subjectType) {
877+
case TPermissionRecord::TTypeCase::USER_ACCOUNT_TYPE:
878+
if (UserAccountService) {
879+
BLOG_TRACE("Ticket " << record.GetMaskedTicket()
880+
<< " asking for UserAccount(" << subjectName << ")");
881+
THolder<TEvAccessServiceGetUserAccountRequest> request = MakeHolder<TEvAccessServiceGetUserAccountRequest>(key);
882+
request->Token = record.Ticket;
883+
request->Request.set_user_account_id(subject.user_account().id());
884+
record.ResponsesLeft++;
885+
Send(UserAccountService, request.Release());
886+
return;
887+
}
888+
break;
889+
case TPermissionRecord::TTypeCase::SERVICE_ACCOUNT_TYPE:
890+
if (ServiceAccountService) {
891+
BLOG_TRACE("Ticket " << record.GetMaskedTicket()
892+
<< " asking for ServiceAccount(" << subjectName << ")");
893+
THolder<TEvAccessServiceGetServiceAccountRequest> request = MakeHolder<TEvAccessServiceGetServiceAccountRequest>(key);
894+
request->Token = record.Ticket;
895+
request->Request.set_service_account_id(subject.service_account().id());
896+
record.ResponsesLeft++;
897+
Send(ServiceAccountService, request.Release());
898+
return;
899+
}
900+
break;
901+
default:
902+
break;
898903
}
899-
SetError(key, record, {.Message = errorMessage, .Retryable = false});
904+
SetToken(request->Key, record, new NACLib::TUserToken(record.Ticket, subjectName, {}));
900905
} else {
901-
SetError(key, record, {.Message = permissionDeniedError, .Retryable = false});
906+
if (hasRequiredPermissionFailed) {
907+
TStringBuilder errorMessage;
908+
auto it = requiredPermissions.cbegin();
909+
errorMessage << (*it)->second.Error.Message;
910+
++it;
911+
for (; it != requiredPermissions.cend(); ++it) {
912+
errorMessage << ", " << (*it)->second.Error.Message;
913+
}
914+
SetError(key, record, {.Message = errorMessage, .Retryable = false});
915+
} else {
916+
SetError(key, record, {.Message = permissionDeniedError, .Retryable = false});
917+
}
902918
}
903919
}
904920
} else {
905-
TString grpsErrorMessage;
906-
bool isRetryableGrpsError = false;
907-
if (response->Response.has_unauthenticated_error()) {
908-
grpsErrorMessage = response->Response.unauthenticated_error().message();
909-
isRetryableGrpsError = false;
910-
} else {
911-
grpsErrorMessage = response->Status.Msg;
912-
isRetryableGrpsError = IsRetryableGrpcError(response->Status);
913-
}
914-
for (auto& [permissionName, permissionRecord] : examinedPermissions) {
915-
permissionRecord.Subject.clear();
916-
permissionRecord.Error = {.Message = grpsErrorMessage, .Retryable = isRetryableGrpsError};
917-
BLOG_TRACE("Ticket " << record.GetMaskedTicket()
918-
<< " permission " << permissionName
919-
<< " now has a " << (isRetryableGrpsError ? "retryable" : "permanent") << " error \"" << grpsErrorMessage << "\""
920-
<< " retryable: " << isRetryableGrpsError);
921-
}
922-
SetError(key, record, {.Message = grpsErrorMessage, .Retryable = isRetryableGrpsError});
921+
SetAccessServiceBulkAuthorizeError(key, record, response->Status.Msg, IsRetryableGrpcError(response->Status));
923922
}
924923
Respond(record);
925924
}

‎ydb/library/testlib/service_mocks/ya.make‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ SRCS(
1212
)
1313

1414
PEERDIR(
15-
# ydb/public/api/client/yc_private/servicecontrol
15+
ydb/public/api/client/yc_private/servicecontrol
1616
ydb/public/api/client/yc_private/accessservice
1717
ydb/public/api/grpc/draft
1818
ydb/public/api/client/yc_private/resourcemanager

‎ydb/library/ycloud/api/ya.make‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ SRCS(
1010

1111
PEERDIR(
1212
ydb/public/api/client/yc_private/iam
13-
# ydb/public/api/client/yc_private/servicecontrol
13+
ydb/public/api/client/yc_private/servicecontrol
1414
ydb/public/api/client/yc_private/accessservice
1515
ydb/public/api/client/yc_private/resourcemanager
1616
ydb/library/actors/core

‎ydb/library/ycloud/impl/mock_access_service.cpp‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
#include <ydb/library/actors/core/actorsystem.h>
22
#include <ydb/library/actors/core/actor.h>
33
#include <library/cpp/json/json_value.h>
4-
#include <ydb/public/api/client/yc_private/accessservice/access_service.grpc.pb.h>
54
#include "access_service.h"
65
#include "grpc_service_client.h"
76
#include "grpc_service_cache.h"

0 commit comments

Comments
 (0)