bug: fix unauth redirect loop

pkg/api/authz/authz.go

@@ -27,7 +27,7 @@ var staticRules = map[string][]string{
 		// Allow access to the UI
 		"/admin/",
 		"/{$}",
-		"/{agent}",
+
 		"/user/images/",
 		"/_app/",
 
@@ -56,8 +56,6 @@ var staticRules = map[string][]string{
 
 		"GET /api/auth-providers",
 		"GET /api/auth-providers/{id}",
-
-		"GET /o/{id}",
 	},
 	AuthenticatedGroup: {
 		"/api/oauth/redirect/{namespace}/{name}",
@@ -85,21 +83,17 @@ var devModeRules = map[string][]string{
 type Authorizer struct {
 	rules        []rule
 	storage      kclient.Client
-	resourcesMux *http.ServeMux
+	apiResources *pathMatcher
+	uiResources  *pathMatcher
 }
 
 func NewAuthorizer(storage kclient.Client, devMode bool) *Authorizer {
-	a := &Authorizer{
+	return &Authorizer{
 		rules:        defaultRules(devMode),
 		storage:      storage,
-		resourcesMux: http.NewServeMux(),
-	}
-
-	for _, resource := range resources {
-		a.resourcesMux.HandleFunc(resource, a.evaluateResources)
+		apiResources: newPathMatcher(apiResources...),
+		uiResources:  newPathMatcher(uiResources...),
 	}
-
-	return a
 }
 
 func (a *Authorizer) Authorize(req *http.Request, user user.Info) bool {
@@ -112,7 +106,11 @@ func (a *Authorizer) Authorize(req *http.Request, user user.Info) bool {
 		}
 	}
 
-	return a.authorizeResource(req, user)
+	if a.authorizeAPIResources(req, user) {
+		return true
+	}
+
+	return a.checkUI(req)
 }
 
 type rule struct {

pkg/api/authz/pathmatcher.go

@@ -0,0 +1,25 @@
+package authz
+
+import "net/http"
+
+type pathMatcher struct {
+	m *http.ServeMux
+}
+
+func newPathMatcher(paths ...string) *pathMatcher {
+	m := http.NewServeMux()
+	for _, path := range paths {
+		m.Handle(path, (*fake)(nil))
+	}
+	return &pathMatcher{m: m}
+}
+
+type GetVar func(string) string
+
+func (p *pathMatcher) Match(req *http.Request) (GetVar, bool) {
+	_, pattern := p.m.Handler(req)
+	if pattern == "" {
+		return nil, false
+	}
+	return req.PathValue, true
+}

pkg/api/authz/resources.go

@@ -1,17 +1,14 @@
 package authz
 
 import (
-	"bytes"
-	"context"
-	"io"
 	"net/http"
+	"slices"
 
 	v1 "github.com/obot-platform/obot/pkg/storage/apis/obot.obot.ai/v1"
-	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apiserver/pkg/authentication/user"
 )
 
-var resources = []string{
+var apiResources = []string{
 	"GET    /api/assistants",
 	"GET    /api/assistants/{assistant_id}",
 	"GET    /api/assistants/{assistant_id}/pending-authorizations",
@@ -133,107 +130,68 @@ type ResourcesAuthorized struct {
 	PendingAuthorization *v1.ThreadAuthorization
 }
 
-func handleError(rw http.ResponseWriter, err error) {
-	if apierrors.IsNotFound(err) {
-		http.Error(rw, err.Error(), http.StatusNotFound)
-	} else if err != nil {
-		http.Error(rw, err.Error(), http.StatusForbidden)
-	} else {
-		rw.WriteHeader(http.StatusForbidden)
-	}
-}
-
-type userKey struct{}
-
-func (a *Authorizer) evaluateResources(rw http.ResponseWriter, req *http.Request) {
-	user, ok := req.Context().Value(userKey{}).(user.Info)
-	if !ok {
-		return
-	}
-
+func (a *Authorizer) evaluateResources(req *http.Request, vars GetVar, user user.Info) (bool, error) {
 	resources := Resources{
-		AssistantID:            req.PathValue("assistant_id"),
-		ProjectID:              req.PathValue("project_id"),
-		ThreadID:               req.PathValue("thread_id"),
-		TemplateID:             req.PathValue("template_id"),
-		TaskID:                 req.PathValue("task_id"),
-		RunID:                  req.PathValue("run_id"),
-		WorkflowID:             req.PathValue("workflow_id"),
-		PendingAuthorizationID: req.PathValue("pending_authorization_id"),
+		AssistantID:            vars("assistant_id"),
+		ProjectID:              vars("project_id"),
+		ThreadID:               vars("thread_id"),
+		TemplateID:             vars("template_id"),
+		TaskID:                 vars("task_id"),
+		RunID:                  vars("run_id"),
+		WorkflowID:             vars("workflow_id"),
+		PendingAuthorizationID: vars("pending_authorization_id"),
 	}
 
 	if ok, err := a.checkAssistant(req, &resources, user); !ok || err != nil {
-		handleError(rw, err)
-		return
+		return false, err
 	}
 
 	if ok, err := a.checkProject(req, &resources, user); !ok || err != nil {
-		handleError(rw, err)
-		return
+		return false, err
 	}
 
 	if ok, err := a.checkThread(req, &resources, user); !ok || err != nil {
-		handleError(rw, err)
-		return
+		return false, err
 	}
 
 	if ok, err := a.checkTemplate(req, &resources, user); !ok || err != nil {
-		handleError(rw, err)
-		return
+		return false, err
 	}
 
 	if ok, err := a.checkTask(req, &resources, user); !ok || err != nil {
-		handleError(rw, err)
-		return
+		return false, err
 	}
 
 	if ok, err := a.checkRun(req, &resources, user); !ok || err != nil {
-		handleError(rw, err)
-		return
+		return false, err
 	}
 
 	if ok, err := a.checkWorkflow(req, &resources, user); !ok || err != nil {
-		handleError(rw, err)
-		return
+		return false, err
 	}
 
 	if ok, err := a.checkPendingAuthorization(req, &resources, user); !ok || err != nil {
-		handleError(rw, err)
-		return
-	}
-
-	if ok, err := a.checkUI(req, &resources, user); !ok || err != nil {
-		handleError(rw, err)
-		return
+		return false, err
 	}
 
-	rw.WriteHeader(http.StatusAccepted)
+	return true, nil
 }
 
-type responseWriter struct {
-	io.Writer
-	code int
-}
-
-func (r *responseWriter) Header() http.Header {
-	return http.Header{}
-}
-
-func (r *responseWriter) WriteHeader(statusCode int) {
-	r.code = statusCode
-}
+func (a *Authorizer) authorizeAPIResources(req *http.Request, user user.Info) bool {
+	vars, matches := a.apiResources.Match(req)
+	if !matches {
+		return false
+	}
 
-func (a *Authorizer) authorizeResource(req *http.Request, user user.Info) bool {
-	h, pattern := a.resourcesMux.Handler(req)
-	if pattern == "" {
+	if !slices.Contains(user.GetGroups(), AuthenticatedGroup) {
+		// All API resources access must be authenticated
 		return false
 	}
 
-	buffer := bytes.NewBuffer(nil)
-	rw := responseWriter{
-		Writer: buffer,
+	ok, err := a.evaluateResources(req, vars, user)
+	if err != nil {
+		return false
 	}
 
-	h.ServeHTTP(&rw, req.WithContext(context.WithValue(req.Context(), userKey{}, user)))
-	return rw.code == http.StatusAccepted
+	return ok
 }

pkg/api/authz/ui.go

@@ -2,15 +2,22 @@ package authz
 
 import (
 	"net/http"
-
-	"k8s.io/apiserver/pkg/authentication/user"
 )
 
-func (a *Authorizer) checkUI(req *http.Request, _ *Resources, _ user.Info) (bool, error) {
-	var ui = req.PathValue("ui")
-	if ui == "" {
-		return true, nil
+var uiResources = []string{
+	"GET /{assistant}",
+	"GET /{assistant}/p/{project}",
+	"GET /o/{project}",
+}
+
+func (a *Authorizer) checkUI(req *http.Request) bool {
+	vars, match := a.uiResources.Match(req)
+	if !match {
+		return false
+	}
+	if vars("assistant") == "api" {
+		return false
 	}
-	// Ensure the URL does not start with /api
-	return ui != "api", nil
+	// Matches and is not API
+	return true
 }

ui/user/src/lib/components/New.svelte

@@ -21,7 +21,7 @@
 				name: newName
 			});
 			const assistant = assistants.items.find((a) => a.id === project.assistantID);
-			window.location.href = `/${assistant?.alias || project.assistantID}/projects/${project.id}`;
+			window.location.href = `/${assistant?.alias || project.assistantID}/p/${project.id}`;
 		}
 		dialog.close();
 	}

ui/user/src/lib/components/navbar/Projects.svelte

@@ -63,7 +63,7 @@
 <div use:tooltip class="flex min-w-[250px] flex-col rounded-3xl bg-surface1 p-2">
 	{#each projects.items as project}
 		<a
-			href="/{getAssistant(project)?.alias || project.assistantID}/projects/{project.id}"
+			href="/{getAssistant(project)?.alias || project.assistantID}/p/{project.id}"
 			rel="external"
 			class="flex items-center gap-2 rounded-3xl p-2 hover:bg-surface2"
 		>