-
Notifications
You must be signed in to change notification settings - Fork 27
adds router posture responses (ha support) #816
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
andrewpmartinez
commented
Oct 6, 2025
- adds ability to submit posture to controller or router based on router capabilities
- adds new ToTP MFA JWT token type for MFA posture checks post-login
- adds ability to submit posture to controller or router based on router capabilities - adds new ToTP MFA JWT token type for MFA posture checks post-login
SdkInfo sdkInfo = 7; | ||
TotpToken totpToken = 8; | ||
}; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the advantage of wrapping this in a type, instead of just using string?
Token: typedResp.TotpToken, | ||
}, | ||
}, | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe add a default
with an ERROR here, so we hopefully notice if a new type is introduced and the case isn't handled yet?
TotpToken string //should be a signed, API Session scoped JWT. See common.TotpClaims | ||
Id *string | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should probably be using pointer receivers, see staticcheck error below
} | ||
return edgeCh | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it make sense to have a container message if you're sending multiple responses?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Over all looks good, left a couple of comments