perf: add face acl failed msg #1613
Conversation
| @@ -244,6 +244,9 @@ func (u *UserSelectHandler) proxyAsset(asset model.PermAsset) { | |||
| } | |||
| tokenInfo = reviewHandler.tokenInfo | |||
| default: | |||
| msg := lang.T("Unknown error code: %s, detail: %s") | |||
| utils.IgnoreErrWriteString(u.h.term, fmt.Sprintf(msg, tokenInfo.Code, tokenInfo.Detail)) | |||
| utils.IgnoreErrWriteString(u.h.term, utils.CharNewLine) | |||
| logger.Errorf("Create connect token and auth info failed: %s %s", tokenInfo.Code, tokenInfo.Detail) | |||
| return | |||
| } | |||
There was a problem hiding this comment.
The code does not contain any known issues or improvements. However, it could be more streamlined by using a single return statement after checking the case for all types of assets. Also, consider if adding a comment at default case provides additional help to someone reading this function, such that they can understand what will happen when no specific case matches. Here is an example:
func (u *UserSelectHandler) proxyAsset(asset model.PermAsset) {
if aclCheckType(u, &asset) && !hasTokenError(&tokenInfo) {This change should improve readability and maintainability without altering functionality.
Let me know if you have another question!
| ACLFaceVerify = "acl_face_verify" | ||
| ACLFaceVerify = "acl_face_verify" | ||
| ACLFaceOnline = "acl_face_online" | ||
| ACLFaceOnlineNotSupported = "acl_face_online_not_supported" | ||
| ) | ||
|
|
||
| type ConnectOptions struct { |
There was a problem hiding this comment.
The code provided appears to be related to an open-source project and is used for connecting various authentication types with specific functions and operations. The first difference I noticed is that the ACLRejected function has been renamed to AclReject. This change suggests there may have been confusion about what each of these values actually represents.
For instance, the documentation indicates two different ways to authenticate user identities:
const ACLReview = "acl_review";
const ACLReject = "acl_reject";
// User reviews an identity based on information from external services
ACLReview();
// User denies a request for approval through a security mechanism such as an ID Verification tool
In contrast, you're referring to:
_ACLFaceVerify = "acl_face_verify"
_ACLFaceOnline = "acl_face_online"
_ACLFaceOnlineNotSupported = "acl_face_online_not_supported"
// A client facing API which can only perform face identification tasks.
_ACLFaceIdentify = "face_auth_identi..."
// An external service which performs face verification tasks (e.g., Google's Image Matching Service).
_ACLFaceVerification ...
// A service which authenticates users' access given their faces, allowing non-biometric methods like photo IDs etc.
_ACLFaceAuth = ...
// Performs authentication via Face Online, without checking if it supports it
_ACLFaceOnline = ""
_ACLFaceOnlineNotSuppor... -- Not supported
// Represents an operation where no further action was taken due to missing support or other factors at this time
_ACLFaceOnlineNotSuport...
// For a service which doesn't handle authentication by default but still requires support of its own algorithms or logic
_ACLFaceUnsupportedService = ""
// For a feature not implemented yet
_ACLFeatureUnimplemented =
This could indicate some sort of miscommunication between authors or maintainers when naming these interfaces/authentications.
I recommend revisiting the design rationale and possibly creating separate interfaces/functions for different aspects rather than conflating everything under one name. For example, ACLFaceVerified, ACLFaceOffline, ACLFaceAuthorized, ACLFaceUnsupported would clearly specify the state of each operation more accurately.
It should help avoid ambiguity and ensure clear, understandable usage.
|
perf: add face acl failed msg