feat: add OTPaaS-backed OTP request/verify endpoints#65
feat: add OTPaaS-backed OTP request/verify endpoints#65kellylimmm wants to merge 18 commits intomainfrom
Conversation
yiningsoong
force-pushed
the
feat/add-otpaas-api-endpoint
branch
from
6d476ff to
d491f98
Compare
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| LogLevel slog.Level `dotenv:"TW_LOG_LEVEL"` | ||
|
|
||
| Server ServerConfig `dotenv:",squash"` | ||
|
|
There was a problem hiding this comment.
nit: Remove new line.
|
|
||
| OTPaas OTPaasConfig `dotenv:",squash"` | ||
|
|
||
| Client ClientConfig `dotenv:",squash"` |
There was a problem hiding this comment.
I don't think we need to have a config just to configure the HTTP client timeout used by OTPaas flow. Either we inline the timeout when creating the HTTP client or place the timeout in OTPassConfig.
| ID string `dotenv:"TW_APP_ID"` | ||
| Namespace string `dotenv:"TW_APP_NAMESPACE"` | ||
| Secret string `dotenv:"TW_APP_SECRET"` | ||
| Host string `dotenv:"TW_TECHPASS_OTP_HOST"` |
There was a problem hiding this comment.
| Host string `dotenv:"TW_TECHPASS_OTP_HOST"` | |
| Host string `dotenv:"TW_OTPASS_HOST"` |
| ) | ||
|
|
||
| func TestIndex(t *testing.T) { | ||
| h := &Handler{cfg: config.Default()} |
There was a problem hiding this comment.
| h := &Handler{cfg: config.Default()} | |
| h := &Handler{} |
Remove cfg since index handler do no need cfg yet.
| } | ||
| } | ||
|
|
||
| func buildAuthToken(appSecret string, appId string, appNamespace string) string { |
There was a problem hiding this comment.
| func buildAuthToken(appSecret string, appId string, appNamespace string) string { | |
| func buildAuthToken(appID string, appNamespace string, appSecret string) string { |
nit: Prefer having the arguments to be ordered like that. Since all 3 arguments have the same type, the first two argument can don't write the type.
|
|
||
| var otpResp OTPResponse | ||
| if err := json.Unmarshal(body, &otpResp); err != nil { | ||
| w.WriteHeader(http.StatusBadGateway) |
There was a problem hiding this comment.
Why the response status is bad gateway but the body is internal server error?
| func (h *Handler) RequestOTP(w http.ResponseWriter, r *http.Request) { | ||
| log := middleware.LoggerFromContext(r.Context()) | ||
| var otpURL = h.cfg.OTPaas.Host + "/otp" | ||
| var sessionID string |
There was a problem hiding this comment.
Place this variable close to where it is being used.
| sessionID = c.Value | ||
| } | ||
|
|
||
| if otpResp.ID == "" { |
There was a problem hiding this comment.
If otpResp.ID is empty, do we still need to read the cookie and generate if it does not exist?
| Name: "session_id", | ||
| Value: sessionID, | ||
| Path: "/", | ||
| Secure: true, |
There was a problem hiding this comment.
We cannot hardcode true to the secure flag. During development, we are using HTTP and not HTTPS so if the secure flag is always true we will not be able to send the cookies for other requests.
We should have an environment variable GO_ENV with values of development and production, similar to how we have NODE_ENV in Javascript and RAILS_ENV in Rails. This way, we can set the secure flag correctly depending on the environment.
| http.SetCookie(w, &cookie) | ||
|
|
||
| w.Header().Set("Content-Type", "application/json") | ||
| w.WriteHeader(http.StatusNoContent) |
There was a problem hiding this comment.
We figured out that we need to send the id to the frontend else we could not show the prefix in the OTP page.
3 participants
🚀 Summary
Adds OTPaaS-backed OTP flows to the server by introducing
/api/otp/requestand/api/otp/verify. The routes use asession_idcookie to persist theotp_flow_id, read OTPaaS credentials from config/env, and return consistent structured JSON error responses (including explicit timeout handling).✏️ Changes
POST /api/otp/request,POST /api/otp/verify).session_idcookie and an in-memory store forotp_flow_id.routes.Register(mux, cfg)).📑 Notes