Skip to content
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.

Sign up for GitHub

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

MM-794: returning proper JSON response from APIs #39

Closed
wants to merge 6 commits into from
Closed

MM-794: returning proper JSON response from APIs #39

Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
aaae417
[MM-794]: returning proper JSON response from APIs
Kshitij-Katiyar Aug 30, 2024
7581658
[MM-794]: fixed the testcases
Kshitij-Katiyar Aug 30, 2024
fc0f3f4
[MM-794]: Review fixes
Kshitij-Katiyar Sep 2, 2024
b1ba237
[MM-794]: minor grammar changes
Kshitij-Katiyar Sep 3, 2024
60015ad
[MM-794]: Fixed testcases
Kshitij-Katiyar Sep 3, 2024
485b3b6
MM-794: review fixes
Kshitij-Katiyar Sep 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
81 changes: 44 additions & 37 deletions server/plugin/api.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,8 @@ func (p *Plugin) checkConfigured(next http.Handler) http.Handler {
config := p.getConfiguration()

if err := config.IsValid(); err != nil {
http.Error(w, "This plugin is not configured.", http.StatusNotImplemented)
p.client.Log.Error("This plugin is not configured.", "error", err)
p.writeAPIError(w, &APIErrorResponse{Message: "this plugin is not configured", StatusCode: http.StatusNotImplemented})
Kshitij-Katiyar marked this conversation as resolved.
Show resolved Hide resolved
return
}

Expand Down Expand Up @@ -298,7 +299,8 @@ func (p *Plugin) connectUserToGitHub(c *Context, w http.ResponseWriter, r *http.

_, err := p.store.Set(githubOauthKey+state.Token, state, pluginapi.SetExpiry(TokenTTL))
if err != nil {
http.Error(w, "error setting stored state", http.StatusBadRequest)
c.Log.WithError(err).Warnf("error occurred while trying to store oauth state into KV store")
p.writeAPIError(w, &APIErrorResponse{Message: "error saving the oauth state", StatusCode: http.StatusInternalServerError})
return
}

Expand Down Expand Up @@ -344,8 +346,8 @@ func (p *Plugin) completeConnectUserToGitHub(c *Context, w http.ResponseWriter,

code := r.URL.Query().Get("code")
if len(code) == 0 {
rErr = errors.New("missing authorization code")
http.Error(w, rErr.Error(), http.StatusBadRequest)
p.client.Log.Error("Missing authorization code.")
p.writeAPIError(w, &APIErrorResponse{Message: "missing authorization code", StatusCode: http.StatusBadRequest})
return
}

Expand All @@ -354,31 +356,26 @@ func (p *Plugin) completeConnectUserToGitHub(c *Context, w http.ResponseWriter,
var state OAuthState
err := p.store.Get(githubOauthKey+stateToken, &state)
if err != nil {
c.Log.Warnf("Failed to get state token", "error", err.Error())

rErr = errors.Wrap(err, "missing stored state")
http.Error(w, rErr.Error(), http.StatusBadRequest)
c.Log.WithError(err).Warnf("error occurred while trying to get oauth state from KV store")
p.writeAPIError(w, &APIErrorResponse{Message: "missing stored state", StatusCode: http.StatusBadRequest})
return
}

err = p.store.Delete(githubOauthKey + stateToken)
if err != nil {
c.Log.WithError(err).Warnf("Failed to delete state token")

rErr = errors.Wrap(err, "error deleting stored state")
http.Error(w, rErr.Error(), http.StatusBadRequest)
c.Log.WithError(err).Warnf("error occurred while trying to delete oauth state from KV store")
p.writeAPIError(w, &APIErrorResponse{Message: "error deleting stored state", StatusCode: http.StatusInternalServerError})
return
}

if state.Token != stateToken {
rErr = errors.New("invalid state token")
http.Error(w, rErr.Error(), http.StatusBadRequest)
p.writeAPIError(w, &APIErrorResponse{Message: "invalid state token", StatusCode: http.StatusBadRequest})
return
}

if state.UserID != c.UserID {
rErr = errors.New("not authorized, incorrect user")
http.Error(w, rErr.Error(), http.StatusUnauthorized)
c.Log.Warnf("not authorized, incorrect user")
p.writeAPIError(w, &APIErrorResponse{Message: "unauthorized user", StatusCode: http.StatusUnauthorized})
return
}

Expand All @@ -390,19 +387,15 @@ func (p *Plugin) completeConnectUserToGitHub(c *Context, w http.ResponseWriter,
tok, err := conf.Exchange(ctx, code)
if err != nil {
c.Log.WithError(err).Warnf("Failed to exchange oauth code into token")

rErr = errors.Wrap(err, "Failed to exchange oauth code into token")
http.Error(w, rErr.Error(), http.StatusInternalServerError)
p.writeAPIError(w, &APIErrorResponse{Message: "failed to exchange oauth code into token", StatusCode: http.StatusInternalServerError})
return
}

githubClient := p.githubConnectToken(*tok)
gitUser, _, err := githubClient.Users.Get(ctx, "")
if err != nil {
c.Log.WithError(err).Warnf("Failed to get authenticated GitHub user")

rErr = errors.Wrap(err, "failed to get authenticated GitHub user")
http.Error(w, rErr.Error(), http.StatusInternalServerError)
p.writeAPIError(w, &APIErrorResponse{Message: "failed to get authenticated GitHub user", StatusCode: http.StatusInternalServerError})
return
}

Expand All @@ -425,9 +418,7 @@ func (p *Plugin) completeConnectUserToGitHub(c *Context, w http.ResponseWriter,

if err = p.storeGitHubUserInfo(userInfo); err != nil {
c.Log.WithError(err).Warnf("Failed to store GitHub user info")

rErr = errors.Wrap(err, "Unable to connect user to GitHub")
http.Error(w, rErr.Error(), http.StatusInternalServerError)
p.writeAPIError(w, &APIErrorResponse{Message: "unable to connect user to GitHub", StatusCode: http.StatusInternalServerError})
return
}

Expand Down Expand Up @@ -512,7 +503,7 @@ func (p *Plugin) completeConnectUserToGitHub(c *Context, w http.ResponseWriter,
_, err = w.Write([]byte(html))
if err != nil {
c.Log.WithError(err).Warnf("Failed to write HTML response")
w.WriteHeader(http.StatusInternalServerError)
p.writeAPIError(w, &APIErrorResponse{Message: "failed to write HTML response", StatusCode: http.StatusInternalServerError})
return
}
}
Expand Down Expand Up @@ -583,7 +574,13 @@ func (p *Plugin) getConnected(c *Context, w http.ResponseWriter, r *http.Request
return
}

info, _ := p.getGitHubUserInfo(c.UserID)
info, err := p.getGitHubUserInfo(c.UserID)
if err != nil {
c.Log.WithError(err).Warnf("failed to get GitHub user info")
p.writeAPIError(w, &APIErrorResponse{Message: "failed to get GitHub user info", StatusCode: http.StatusInternalServerError})
return
}

if info == nil || info.Token == nil {
p.writeJSON(w, resp)
return
Expand All @@ -597,10 +594,14 @@ func (p *Plugin) getConnected(c *Context, w http.ResponseWriter, r *http.Request
if info.Settings.DailyReminder && r.URL.Query().Get("reminder") == "true" {
lastPostAt := info.LastToDoPostAt

var timezone *time.Location
offset, _ := strconv.Atoi(r.Header.Get("X-Timezone-Offset"))
timezone = time.FixedZone("local", -60*offset)
offset, err := strconv.Atoi(r.Header.Get("X-Timezone-Offset"))
if err != nil {
c.Log.WithError(err).Warnf("Invalid timezone offset")
p.writeAPIError(w, &APIErrorResponse{Message: "invalid timezone offset", StatusCode: http.StatusBadRequest})
return
}

timezone := time.FixedZone("local", -60*offset)
// Post to do message if it's the next day and been more than an hour since the last post
now := model.GetMillis()
nt := time.Unix(now/1000, 0).In(timezone)
Expand All @@ -623,6 +624,7 @@ func (p *Plugin) getConnected(c *Context, w http.ResponseWriter, r *http.Request
var val []byte
err := p.store.Get(privateRepoStoreKey, &val)
if err != nil {
p.writeAPIError(w, &APIErrorResponse{Message: "Unable to get private repo key value", StatusCode: http.StatusInternalServerError})
c.Log.WithError(err).Warnf("Unable to get private repo key value")
return
}
Expand All @@ -635,8 +637,8 @@ func (p *Plugin) getConnected(c *Context, w http.ResponseWriter, r *http.Request
} else {
p.CreateBotDMPost(info.UserID, fmt.Sprintf(message, "`/github connect private`."), "")
}
_, err := p.store.Set(privateRepoStoreKey, []byte("1"))
if err != nil {
if _, err := p.store.Set(privateRepoStoreKey, []byte("1")); err != nil {
p.writeAPIError(w, &APIErrorResponse{Message: "unable to set private repo key value", StatusCode: http.StatusInternalServerError})
c.Log.WithError(err).Warnf("Unable to set private repo key value")
}
}
Expand All @@ -653,6 +655,7 @@ func (p *Plugin) getMentions(c *UserContext, w http.ResponseWriter, r *http.Requ

result, _, err := githubClient.Search.Issues(c.Ctx, query, &github.SearchOptions{})
if err != nil {
p.writeAPIError(w, &APIErrorResponse{Message: "failed to search for issues", StatusCode: http.StatusInternalServerError})
c.Log.WithError(err).With(logger.LogContext{"query": query}).Warnf("Failed to search for issues")
return
}
Expand Down Expand Up @@ -971,6 +974,7 @@ func (p *Plugin) getSidebarContent(c *UserContext, w http.ResponseWriter, r *htt
sidebarContent, err := p.getSidebarData(c)
if err != nil {
c.Log.WithError(err).Warnf("Failed to search for the sidebar data")
p.writeAPIError(w, &APIErrorResponse{Message: "failed to search for the sidebar data", StatusCode: http.StatusInternalServerError})
return
}

Expand Down Expand Up @@ -1001,12 +1005,13 @@ func (p *Plugin) updateSettings(c *UserContext, w http.ResponseWriter, r *http.R
var settings *UserSettings
if err := json.NewDecoder(r.Body).Decode(&settings); err != nil {
c.Log.WithError(err).Warnf("Error decoding settings from JSON body")
http.Error(w, "Invalid request body", http.StatusBadRequest)
p.writeAPIError(w, &APIErrorResponse{Message: "invalid request body", StatusCode: http.StatusBadRequest})
return
}

if settings == nil {
http.Error(w, "Invalid request body", http.StatusBadRequest)
p.client.Log.Error("Invalid request body.")
p.writeAPIError(w, &APIErrorResponse{Message: "invalid request body", StatusCode: http.StatusBadRequest})
return
}

Expand All @@ -1015,7 +1020,7 @@ func (p *Plugin) updateSettings(c *UserContext, w http.ResponseWriter, r *http.R

if err := p.storeGitHubUserInfo(info); err != nil {
c.Log.WithError(err).Warnf("Failed to store GitHub user info")
http.Error(w, "Encountered error updating settings", http.StatusInternalServerError)
p.writeAPIError(w, &APIErrorResponse{Message: "error occurred while updating settings", StatusCode: http.StatusInternalServerError})
return
}

Expand Down Expand Up @@ -1437,13 +1442,15 @@ func (p *Plugin) getConfig(w http.ResponseWriter, r *http.Request) {
func (p *Plugin) getToken(w http.ResponseWriter, r *http.Request) {
userID := r.FormValue("userID")
if userID == "" {
http.Error(w, "please provide a userID", http.StatusBadRequest)
p.client.Log.Error("UserID not found.")
raghavaggarwal2308 marked this conversation as resolved.
Show resolved Hide resolved
p.writeAPIError(w, &APIErrorResponse{Message: "please provide a userID", StatusCode: http.StatusBadRequest})
return
}

info, apiErr := p.getGitHubUserInfo(userID)
if apiErr != nil {
http.Error(w, apiErr.Error(), apiErr.StatusCode)
p.client.Log.Error("error occurred while getting the github user info", "UserID", userID, "error", apiErr)
p.writeAPIError(w, &APIErrorResponse{Message: apiErr.Error(), StatusCode: apiErr.StatusCode})
return
}

Expand Down
Loading