canonical / identity-platform-admin-ui

Admin UI for the Canonical identity broker and identity provider solution
Other
5 stars 4 forks source link

Add `chi` side validation for URLs + small fixes needed in `converters.go` #239

Closed BarcoMasile closed 3 months ago

BarcoMasile commented 3 months ago

Description

In internal/authorization/converters.go, L228, one thing we don't do here is actually check for the first id (group_id) to be non-blank, may raise issues at runtime if we don't validate endpoints path variables before this middleware is run, otherwise we would need to first check after parameter retrieval if the group_id is empty and the method allows for an empty parameter, like GET ( -> can_view) or POST ( -> can_edit).

Using regexp URL parameters to validate that selected endpoints have an ID path variable populated is the best way to fix this. From the doc:

  // Regexp url parameters:
  r.Get("/{articleSlug:[a-z-]+}", getArticleBySlug) // GET /articles/home-is-toronto

Bonus nitpick

/entitlements suffixed endpoints for group don't support POST operations in this PR, only GET, PATCH and DELETE

syncronize-issues-to-jira[bot] commented 3 months ago

Thank you for reporting us your feedback!

The internal ticket has been created: https://warthogs.atlassian.net/browse/IAM-769.

This message was autogenerated