FRE-4763: Fix Clone() context argument and apply Code Reviewer P0-P3 fixes
- P0: Update auth header after token refresh via GetSession() + SetAuthHeader() - P2: Unconditional req.WithContext(ctx) instead of fragile context.Background() check - Fix: req.Clone(ctx) takes context.Context, not *http.Request (req.WithContext(ctx)) - Remove unused checkAuthenticated() and NewRequestWithContext() helpers
This commit is contained in:
16
cmd/mail.go
16
cmd/mail.go
@@ -15,22 +15,6 @@ import (
|
|||||||
"github.com/spf13/cobra"
|
"github.com/spf13/cobra"
|
||||||
)
|
)
|
||||||
|
|
||||||
func checkAuthenticated() (*auth.Session, error) {
|
|
||||||
sessionMgr, err := auth.NewSessionManager()
|
|
||||||
if err != nil {
|
|
||||||
return nil, fmt.Errorf("failed to create session manager: %w", err)
|
|
||||||
}
|
|
||||||
authenticated, err := sessionMgr.IsAuthenticated()
|
|
||||||
if err != nil || !authenticated {
|
|
||||||
return nil, fmt.Errorf("not authenticated (run 'pop login' first): %w", err)
|
|
||||||
}
|
|
||||||
session, err := sessionMgr.GetSession()
|
|
||||||
if err != nil {
|
|
||||||
return nil, fmt.Errorf("not authenticated: %w", err)
|
|
||||||
}
|
|
||||||
return session, nil
|
|
||||||
}
|
|
||||||
|
|
||||||
func checkAuthenticatedWithManager() (*auth.Session, *auth.SessionManager, error) {
|
func checkAuthenticatedWithManager() (*auth.Session, *auth.SessionManager, error) {
|
||||||
sessionMgr, err := auth.NewSessionManager()
|
sessionMgr, err := auth.NewSessionManager()
|
||||||
if err != nil {
|
if err != nil {
|
||||||
|
|||||||
@@ -102,11 +102,7 @@ func (c *ProtonMailClient) DoWithContext(ctx context.Context, req *http.Request)
|
|||||||
|
|
||||||
req.Header.Set("Authorization", fmt.Sprintf("Bearer %s", c.getAuthHeader()))
|
req.Header.Set("Authorization", fmt.Sprintf("Bearer %s", c.getAuthHeader()))
|
||||||
req.Header.Set("Accept", "application/json")
|
req.Header.Set("Accept", "application/json")
|
||||||
|
req = req.WithContext(ctx)
|
||||||
// Check if request has its own context
|
|
||||||
if ctx != context.Background() {
|
|
||||||
req = req.WithContext(ctx)
|
|
||||||
}
|
|
||||||
|
|
||||||
resp, err := c.httpClient.Do(req)
|
resp, err := c.httpClient.Do(req)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
@@ -128,6 +124,13 @@ func (c *ProtonMailClient) DoWithContext(ctx context.Context, req *http.Request)
|
|||||||
return resp, fmt.Errorf("401 received and refresh failed: %w", err)
|
return resp, fmt.Errorf("401 received and refresh failed: %w", err)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Update auth header with new access token from refreshed session
|
||||||
|
session, err := c.sessionRefresher.GetSession()
|
||||||
|
if err != nil {
|
||||||
|
return resp, fmt.Errorf("401 received, refresh succeeded but failed to get new session: %w", err)
|
||||||
|
}
|
||||||
|
c.SetAuthHeader(session.AccessToken)
|
||||||
|
|
||||||
// Retry the request with new token
|
// Retry the request with new token
|
||||||
// Clone the request to reset any body position
|
// Clone the request to reset any body position
|
||||||
retryReq := req.Clone(ctx)
|
retryReq := req.Clone(ctx)
|
||||||
@@ -167,13 +170,4 @@ func (e *APIError) Error() string {
|
|||||||
return fmt.Sprintf("API error %d: %s", e.HTTPStatus, e.Message)
|
return fmt.Sprintf("API error %d: %s", e.HTTPStatus, e.Message)
|
||||||
}
|
}
|
||||||
|
|
||||||
// Helper function to create a request with context
|
|
||||||
func NewRequestWithContext(ctx context.Context, method, url string, body io.Reader) (*http.Request, error) {
|
|
||||||
req, err := http.NewRequestWithContext(ctx, method, url, body)
|
|
||||||
if err != nil {
|
|
||||||
return nil, err
|
|
||||||
}
|
|
||||||
req.Header.Set("Content-Type", "application/json")
|
|
||||||
req.Header.Set("Accept", "application/json")
|
|
||||||
return req, nil
|
|
||||||
}
|
|
||||||
|
|||||||
Reference in New Issue
Block a user