Skip to content

Commit d684545

Browse files
committed
restore explanatory comments in ProcessData
Address review feedback: the prior commits removed too many comments from ProcessData. Restore the ones that explain non-obvious intent (cross-map deletion rationale, defensive-check notes, ambiguous-ID clearing logic) while dropping the genuinely redundant ones. Made-with: Cursor
1 parent 6fc2ccb commit d684545

File tree

1 file changed

+7
-0
lines changed
  • pkg/detectors/azure_entra/serviceprincipal/v2

1 file changed

+7
-0
lines changed

pkg/detectors/azure_entra/serviceprincipal/v2/spv2.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,7 @@ SecretLoop:
101101
}
102102
tenantId = tId
103103

104+
// Skip known-invalid client/tenant combinations.
104105
invalidClients := invalidClientsForTenant[tenantId]
105106
if invalidClients == nil {
106107
invalidClients = map[string]struct{}{}
@@ -112,9 +113,11 @@ SecretLoop:
112113

113114
if verify {
114115
if !azure_entra.TenantExists(logCtx, client, tenantId) {
116+
// Tenant doesn't exist.
115117
delete(activeTenants, tenantId)
116118
continue
117119
}
120+
// Tenant exists, ensure this isn't attempted as a clientId.
118121
delete(activeClients, tenantId)
119122

120123
isVerified, extraData, verificationErr := serviceprincipal.VerifyCredentials(ctx, client, tenantId, clientId, clientSecret)
@@ -127,14 +130,17 @@ SecretLoop:
127130
case errors.Is(verificationErr, serviceprincipal.ErrSecretExpired):
128131
continue SecretLoop
129132
case errors.Is(verificationErr, serviceprincipal.ErrTenantNotFound):
133+
// Tenant doesn't exist; shouldn't happen given the check above.
130134
delete(activeTenants, tenantId)
131135
continue
132136
case errors.Is(verificationErr, serviceprincipal.ErrClientNotFoundInTenant):
137+
// Tenant is valid but the client ID doesn't exist in it.
133138
invalidClients[clientId] = struct{}{}
134139
continue
135140
}
136141
}
137142

143+
// The result is verified or there's only one associated client and tenant.
138144
if isVerified || (len(activeClients) == 1 && len(activeTenants) == 1) {
139145
r = createResult(tenantId, clientId, clientSecret, isVerified, extraData, verificationErr)
140146
break ClientLoop
@@ -144,6 +150,7 @@ SecretLoop:
144150
}
145151

146152
if r == nil {
153+
// Only include the clientId and tenantId if we're confident which one it is.
147154
if len(activeClients) != 1 {
148155
clientId = ""
149156
}

0 commit comments

Comments
 (0)