fix Content-Type header not being set for error responses

Replace WriteHeader + RenderJSON pattern with EncodeJSON to ensure
Content-Type: application/json header is set before WriteHeader is called.
This fixes go-pkgz/rest#38 where responses had text/plain content-type.

Author: paskal

_example/main.go

@@ -49,7 +49,7 @@ func main() {
 		AvatarStore:       avatar.NewLocalFS("/tmp/demo-auth-service"), // stores avatars locally
 		AvatarResizeLimit: 200,                                         // resizes avatars to 200x200
 		ClaimsUpd: token.ClaimsUpdFunc(func(claims token.Claims) token.Claims { // modify issued token
-			if claims.User != nil && claims.User.Name == "dev_admin" {          // set attributes for dev_admin
+			if claims.User != nil && claims.User.Name == "dev_admin" { // set attributes for dev_admin
 				claims.User.SetAdmin(true)
 				claims.User.SetStrAttr("custom-key", "some value")
 			}
@@ -144,7 +144,7 @@ func main() {
 		devAuthServer.Run(context.Background())
 	}()
 
-	// Example: start custom oauth2 server, add to handlers
+	// example: start custom oauth2 server, add to handlers
 	srv := initGoauth2Srv()
 	sopts := provider.CustomServerOpt{
 		URL:           "http://127.0.0.1:9096",
@@ -154,11 +154,11 @@ func main() {
 	// create custom provider and prepare params for handler
 	prov := provider.NewCustomServer(srv, sopts)
 
-	// Start server
+	// start server
 	go prov.Run(context.Background())
 	service.AddCustomProvider("custom123", auth.Client{Cid: "cid", Csecret: "csecret"}, prov.HandlerOpt)
 
-	// Example: add different oauth2 provider
+	// example: add different oauth2 provider
 	c := auth.Client{
 		Cid:     os.Getenv("AEXMPL_BITBUCKET_CID"),
 		Csecret: os.Getenv("AEXMPL_BITBUCKET_CSEC"),

auth.go

@@ -172,8 +172,7 @@ func (s *Service) Handlers() (authHandler, avatarHandler http.Handler) {
 		// allow logout without specifying provider
 		if elems[len(elems)-1] == "logout" {
 			if len(s.providers) == 0 {
-				w.WriteHeader(http.StatusBadRequest)
-				rest.RenderJSON(w, rest.JSON{"error": "providers not defined"})
+				_ = rest.EncodeJSON(w, http.StatusBadRequest, rest.JSON{"error": "providers not defined"})
 				return
 			}
 			s.providers[0].Handler(w, r)
@@ -184,12 +183,11 @@ func (s *Service) Handlers() (authHandler, avatarHandler http.Handler) {
 		if elems[len(elems)-1] == "user" {
 			claims, _, err := s.jwtService.Get(r)
 			if err != nil || claims.User == nil {
-				w.WriteHeader(http.StatusUnauthorized)
 				msg := "user is nil"
 				if err != nil {
 					msg = err.Error()
 				}
-				rest.RenderJSON(w, rest.JSON{"error": msg})
+				_ = rest.EncodeJSON(w, http.StatusUnauthorized, rest.JSON{"error": msg})
 				return
 			}
 			rest.RenderJSON(w, claims.User)
@@ -211,8 +209,7 @@ func (s *Service) Handlers() (authHandler, avatarHandler http.Handler) {
 		provName := elems[len(elems)-2]
 		p, err := s.Provider(provName)
 		if err != nil {
-			w.WriteHeader(http.StatusBadRequest)
-			rest.RenderJSON(w, rest.JSON{"error": fmt.Sprintf("provider %s not supported", provName)})
+			_ = rest.EncodeJSON(w, http.StatusBadRequest, rest.JSON{"error": fmt.Sprintf("provider %s not supported", provName)})
 			return
 		}
 		p.Handler(w, r)
@@ -347,7 +344,7 @@ func (s *Service) AddAppleProvider(appleConfig provider.AppleConfig, privKeyLoad
 		L:           s.logger,
 	}
 
-	// Error checking at create need for catch one when apple private key init
+	// error checking at create need for catch one when apple private key init
 	appleProvider, err := provider.NewApple(p, appleConfig, privKeyLoader)
 	if err != nil {
 		return fmt.Errorf("an AppleProvider creating failed: %w", err)

avatar/avatar.go

@@ -194,7 +194,7 @@ func (p *Proxy) resize(reader io.Reader, limit int) io.Reader {
 		newW, newH = limit, h*limit/w
 	}
 	m := image.NewRGBA(image.Rect(0, 0, newW, newH))
-	// Slower than `draw.ApproxBiLinear.Scale()` but better quality.
+	// slower than `draw.ApproxBiLinear.Scale()` but better quality.
 	draw.BiLinear.Scale(m, m.Bounds(), src, src.Bounds(), draw.Src, nil)
 
 	var out bytes.Buffer

avatar/avatar_test.go

@@ -252,16 +252,16 @@ func TestAvatar_resize(t *testing.T) {
 	}
 
 	p := Proxy{L: logger.Std}
-	// Reader is nil.
+	// reader is nil.
 	resizedR := p.resize(nil, 100)
 	assert.Nil(t, resizedR)
 
-	// Negative limit error.
+	// negative limit error.
 	resizedR = p.resize(strings.NewReader("some picture bin data"), -1)
 	require.NotNil(t, resizedR)
 	checkC(t, resizedR, []byte("some picture bin data"))
 
-	// Decode error.
+	// decode error.
 	resizedR = p.resize(strings.NewReader("invalid image content"), 100)
 	assert.NotNil(t, resizedR)
 	checkC(t, resizedR, []byte("invalid image content"))
@@ -278,12 +278,12 @@ func TestAvatar_resize(t *testing.T) {
 		img, err := os.ReadFile(c.file)
 		require.Nil(t, err, "can't open test file %s", c.file)
 
-		// No need for resize, avatar dimensions are smaller than resize limit.
+		// no need for resize, avatar dimensions are smaller than resize limit.
 		resizedR = p.resize(bytes.NewReader(img), 800)
 		assert.NotNil(t, resizedR, "file %s", c.file)
 		checkC(t, resizedR, img)
 
-		// Resizing to half of width. Check resizedR avatar format PNG.
+		// resizing to half of width. Check resizedR avatar format PNG.
 		resizedR = p.resize(bytes.NewReader(img), 400)
 		assert.NotNil(t, resizedR, "file %s", c.file)
 

middleware/auth.go

@@ -189,7 +189,7 @@ func (a *Authenticator) refreshExpiredToken(w http.ResponseWriter, claims token.
 	}
 
 	claims.ExpiresAt = 0                  // this will cause now+duration for refreshed token
-	c, err := a.JWTService.Set(w, claims) // Set changes token
+	c, err := a.JWTService.Set(w, claims) // set changes token
 	if err != nil {
 		return token.Claims{}, err
 	}

provider/apple.go

@@ -49,22 +49,22 @@ const (
 
 // appleVerificationResponse is based on https://developer.apple.com/documentation/signinwithapplerestapi/tokenresponse
 type appleVerificationResponse struct {
-	// A token used to access allowed user data, but now not implemented public interface for it.
+	// a token used to access allowed user data, but now not implemented public interface for it.
 	AccessToken string `json:"access_token"`
 
-	// Access token type, always equal the "bearer".
+	// access token type, always equal the "bearer".
 	TokenType string `json:"token_type"`
 
-	// Access token expires time in seconds. Always equal 3600 seconds (1 hour)
+	// access token expires time in seconds. Always equal 3600 seconds (1 hour)
 	ExpiresIn int `json:"expires_in"`
 
-	// The refresh token used to regenerate new access tokens.
+	// the refresh token used to regenerate new access tokens.
 	RefreshToken string `json:"refresh_token"`
 
-	// Main JSON Web Token that contains the user’s identity information.
+	// main JSON Web Token that contains the user’s identity information.
 	IDToken string `json:"id_token"`
 
-	// Used to capture any error returned in response. Always check error for empty
+	// used to capture any error returned in response. Always check error for empty
 	Error string `json:"error"`
 }
 
@@ -443,7 +443,7 @@ func (ah *AppleHandler) exchange(ctx context.Context, code, redirectURI string,
 		return err
 	}
 
-	// Trying to decode (unmarshal json) data of response
+	// trying to decode (unmarshal json) data of response
 	err = json.NewDecoder(res.Body).Decode(result)
 	if err != nil {
 		return fmt.Errorf("unmarshalling data from apple service response failed: %w", err)
@@ -455,8 +455,8 @@ func (ah *AppleHandler) exchange(ctx context.Context, code, redirectURI string,
 		}
 	}()
 
-	// If above operation done successfully checking a response code and error descriptions, if one exist.
-	// Apple service will response either 200 (OK) or 400 (any error).
+	// if above operation done successfully checking a response code and error descriptions, if one exist.
+	// apple service will response either 200 (OK) or 400 (any error).
 	if res.StatusCode != http.StatusOK || result.Error != "" {
 		return fmt.Errorf("apple token service error: %s", result.Error)
 	}
@@ -471,7 +471,7 @@ func (ah *AppleHandler) createClientSecret() (string, error) {
 	if ah.conf.privateKey == nil {
 		return "", fmt.Errorf("private key can't be empty")
 	}
-	// Create a claims
+	// create a claims
 	now := time.Now()
 	exp := now.Add(time.Minute * 30).Unix() // default value
 
@@ -502,7 +502,7 @@ func (ah *AppleHandler) parseUserData(user *token.User, jUser string) {
 
 	var userData UserData
 
-	// Catch error for log only. No need break flow if user name doesn't exist
+	// catch error for log only. No need break flow if user name doesn't exist
 	if err := json.Unmarshal([]byte(jUser), &userData); err != nil {
 		ah.Logf("[DEBUG] failed to parse user data %s: %v", user, err)
 		user.Name = "noname_" + user.ID[6:12] // paste noname if user name failed to parse

provider/custom_server_test.go

@@ -51,7 +51,7 @@ func TestCustomProvider(t *testing.T) {
 		L:             logger.Std,
 		WithLoginPage: true,
 		LoginPageHandler: func(w http.ResponseWriter, r *http.Request) {
-			// // Simulate POST from login page
+			// // simulate POST from login page
 			u, err := url.Parse("http://127.0.0.1:9096/authorize?" + r.URL.RawQuery)
 			if err != nil {
 				assert.Fail(t, "failed to parse url")

provider/sender/email.go

@@ -20,14 +20,14 @@ type Email struct {
 type EmailParams struct {
 	Host        string // SMTP host
 	Port        int    // SMTP port
-	From        string // From email field
-	Subject     string // Email subject
-	ContentType string // Content type
+	From        string // from email field
+	Subject     string // email subject
+	ContentType string // content type
 
 	TLS                bool          // TLS auth
-	StartTLS           bool          // StartTLS auth
-	InsecureSkipVerify bool          // Skip certificate verification
-	Charset            string        // Character set
+	StartTLS           bool          // startTLS auth
+	InsecureSkipVerify bool          // skip certificate verification
+	Charset            string        // character set
 	LoginAuth          bool          // LOGIN auth method instead of default PLAIN, needed for Office 365 and outlook.com
 	SMTPUserName       string        // username
 	SMTPPassword       string        // password

provider/telegram.go

@@ -63,7 +63,7 @@ var expiredCleanupInterval = time.Minute * 5 // interval to check and clean up e
 // Run starts processing login requests sent in Telegram
 // Blocks caller
 func (th *TelegramHandler) Run(ctx context.Context) error {
-	// Initialization
+	// initialization
 	atomic.AddInt32(&th.run, 1)
 	info, err := th.Telegram.BotInfo(ctx)
 	if err != nil {
@@ -168,7 +168,7 @@ func (th *TelegramHandler) processUpdates(ctx context.Context, updates *telegram
 
 		th.requests.RLock()
 		authRequest, ok := th.requests.data[token]
-		if !ok { // No such token
+		if !ok { // no such token
 			th.requests.RUnlock()
 			err := th.Telegram.Send(ctx, update.Message.Chat.ID, th.ErrorMsg)
 			if err != nil {
@@ -256,7 +256,7 @@ func (th *TelegramHandler) LoginHandler(w http.ResponseWriter, r *http.Request)
 	queryToken := r.URL.Query().Get("token")
 	if queryToken == "" {
 		// GET /login (No token supplied)
-		// Generate and send token
+		// generate and send token
 		token, err := randToken()
 		if err != nil {
 			rest.SendErrorJSON(w, r, th.L, http.StatusInternalServerError, err, "failed to generate code")
@@ -322,7 +322,7 @@ func (th *TelegramHandler) LoginHandler(w http.ResponseWriter, r *http.Request)
 
 	rest.RenderJSON(w, claims.User)
 
-	// Delete request
+	// delete request
 	th.requests.Lock()
 	defer th.requests.Unlock()
 	delete(th.requests.data, queryToken)
@@ -342,8 +342,8 @@ type tgAPI struct {
 	token  string
 	client *http.Client
 
-	// Identifier of the first update to be requested.
-	// Should be equal to LastSeenUpdateID + 1
+	// identifier of the first update to be requested.
+	// should be equal to LastSeenUpdateID + 1
 	// See https://core.telegram.org/bots/api#getupdates
 	updateOffset int
 }
@@ -387,7 +387,7 @@ func (tg *tgAPI) Send(ctx context.Context, id int, msg string) error {
 
 // Avatar returns URL to user avatar
 func (tg *tgAPI) Avatar(ctx context.Context, id int) (string, error) {
-	// Get profile pictures
+	// get profile pictures
 	url := fmt.Sprintf(`getUserProfilePhotos?user_id=%d`, id)
 
 	var profilePhotos = struct {
@@ -402,12 +402,12 @@ func (tg *tgAPI) Avatar(ctx context.Context, id int) (string, error) {
 		return "", err
 	}
 
-	// User does not have profile picture set or it is hidden in privacy settings
+	// user does not have profile picture set or it is hidden in privacy settings
 	if len(profilePhotos.Result.Photos) == 0 || len(profilePhotos.Result.Photos[0]) == 0 {
 		return "", nil
 	}
 
-	// Get max possible picture size
+	// get max possible picture size
 	last := len(profilePhotos.Result.Photos[0]) - 1
 	fileID := profilePhotos.Result.Photos[0][last].ID
 	url = fmt.Sprintf(`getFile?file_id=%s`, fileID)

provider/telegram_test.go

@@ -50,7 +50,7 @@ func TestTelegramUnconfirmedRequest(t *testing.T) {
 	tg, cleanup := setupHandler(t, m)
 	defer cleanup()
 
-	// Get token
+	// get token
 	r := httptest.NewRequest("GET", "/", nil)
 	w := httptest.NewRecorder()
 	tg.LoginHandler(w, r)
@@ -68,7 +68,7 @@ func TestTelegramUnconfirmedRequest(t *testing.T) {
 	assert.Equal(t, "my_auth_bot", resp.Bot)
 	token := resp.Token
 
-	// Make sure we get error without first confirming auth request
+	// make sure we get error without first confirming auth request
 	r = httptest.NewRequest("GET", fmt.Sprintf("/?token=%s", token), nil)
 	w = httptest.NewRecorder()
 	tg.LoginHandler(w, r)
@@ -78,7 +78,7 @@ func TestTelegramUnconfirmedRequest(t *testing.T) {
 
 	time.Sleep(tgAuthRequestLifetime)
 
-	// Confirm auth request expired
+	// confirm auth request expired
 	r = httptest.NewRequest("GET", fmt.Sprintf("/?token=%s", token), nil)
 	w = httptest.NewRecorder()
 	tg.LoginHandler(w, r)
@@ -146,7 +146,7 @@ func TestTelegramConfirmedRequest(t *testing.T) {
 	tg, cleanup := setupHandler(t, m)
 	defer cleanup()
 
-	// Get token
+	// get token
 	r := httptest.NewRequest("GET", "/", nil)
 	w := httptest.NewRecorder()
 	tg.LoginHandler(w, r)
@@ -166,7 +166,7 @@ func TestTelegramConfirmedRequest(t *testing.T) {
 	servedToken = resp.Token
 	wgToken.Done()
 
-	// Check the token confirmation
+	// check the token confirmation
 	assert.Eventually(t, func() bool {
 		r = httptest.NewRequest("GET", fmt.Sprintf("/?token=%s", resp.Token), nil)
 		w = httptest.NewRecorder()
@@ -186,7 +186,7 @@ func TestTelegramConfirmedRequest(t *testing.T) {
 	assert.Contains(t, info.ID, "telegram_")
 	assert.Equal(t, "http://example.com/ava12345.png", info.Picture)
 
-	// Test request has been invalidated
+	// test request has been invalidated
 	r = httptest.NewRequest("GET", fmt.Sprintf("/?token=%s", resp.Token), nil)
 	w = httptest.NewRecorder()
 	tg.LoginHandler(w, r)
@@ -206,7 +206,7 @@ func TestTelegramLogout(t *testing.T) {
 	tg, cleanup := setupHandler(t, m)
 	defer cleanup()
 
-	// Same TestVerifyHandler_Logout
+	// same TestVerifyHandler_Logout
 	handler := http.HandlerFunc(tg.LogoutHandler)
 	rr := httptest.NewRecorder()
 	req, err := http.NewRequest("GET", "/logout", http.NoBody)
@@ -292,7 +292,7 @@ func TestTelegram_ProcessUpdateFlow(t *testing.T) {
 	assert.EqualError(t, tg.ProcessUpdate(context.Background(), ""), "failed to decode provided telegram update: unexpected end of JSON input")
 	assert.Len(t, tg.requests.data, 1, "expired token should be cleaned up despite the error")
 
-	// Verify that get token will return bot name
+	// verify that get token will return bot name
 	r := httptest.NewRequest("GET", "/", nil)
 	w := httptest.NewRecorder()
 	tg.LoginHandler(w, r)