Skip to content

[API-75] Implement full requireAuthMiddleware#51

Merged
raymondjacobson merged 2 commits intomainfrom
rj-api-75
Apr 25, 2025
Merged

[API-75] Implement full requireAuthMiddleware#51
raymondjacobson merged 2 commits intomainfrom
rj-api-75

Conversation

@raymondjacobson
Copy link
Copy Markdown
Member

Further implements requireAuthMiddleware to return appropriate 401 and 403 responses (including manager mode support)

Copy link
Copy Markdown
Contributor

@stereosteve stereosteve left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

@raymondjacobson raymondjacobson merged commit 1d3c9f4 into main Apr 25, 2025
2 checks passed
@raymondjacobson raymondjacobson deleted the rj-api-75 branch April 25, 2025 16:27
Comment thread api/server.go
panic(err)
}

resolveGrantCache, err := otter.MustBuilder[string, bool](50_000).
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Am I wrong or does this mean when you delete a grant we will take 10 minutes to stop honoring it for the manager/app in question?
Seems like that might create a concerning UX or folks who expect the revocation to be immediate.

Comment thread api/auth_middleware.go
AND is_approved = true
AND is_revoked = false
)
`, userId, authedWallet).Scan(&isAuthorized)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this prone to any casing issues or did we fix that already?

Comment thread api/auth_middleware.go
return c.Next()
}

if wallet != "" && wallet == authedWallet {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this meant as a catch-all for endpoints that use a wallet param somewhere in their URL?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants