Merge pull request #9678 from reasonerjt/refine-req-handle-1.9

Refine request handle process

make/photon/prepare/templates/core/app.conf.jinja

@@ -4,3 +4,6 @@ enablegzip = true
 
 [dev]
 httpport = 8080
+EnableXSRF = true
+XSRFKey = {{xsrf_key}}
+XSRFExpire = 3600

make/photon/prepare/utils/core.py

@@ -35,8 +35,13 @@ def prepare_core(config_dict, with_notary, with_clair, with_chartmuseum):
         with_chartmuseum=with_chartmuseum,
         **config_dict)
 
-    # Copy Core app.conf
-    copy_core_config(core_conf_template_path, core_conf)
+    render_jinja(
+        core_conf_template_path,
+        core_conf,
+        uid=DEFAULT_UID,
+        gid=DEFAULT_GID,
+        xsrf_key=generate_random_string(40))
+
 
 
 def copy_core_config(core_templates_path, core_config_path):

src/core/api/base.go

@@ -83,6 +83,10 @@ func (b *BaseController) Prepare() {
 		return
 	}
 	b.ProjectMgr = pm
+
+	if !filter.ReqCarriesSession(b.Ctx.Request) {
+		b.EnableXSRF = false
+	}
 }
 
 // RequireAuthenticated returns true when the request is authenticated

src/core/api/harborapi_test.go

@@ -92,6 +92,7 @@ func init() {
 	beego.TestBeegoInit(apppath)
 
 	filter.Init()
+	beego.InsertFilter("/api/*", beego.BeforeStatic, filter.SessionCheck)
 	beego.InsertFilter("/*", beego.BeforeRouter, filter.SecurityFilter)
 
 	beego.Router("/api/health", &HealthAPI{}, "get:CheckHealth")

src/core/api/user.go

@@ -17,6 +17,7 @@ package api
 import (
 	"errors"
 	"fmt"
+	"github.com/goharbor/harbor/src/core/filter"
 	"net/http"
 	"regexp"
 	"strconv"
@@ -317,6 +318,11 @@ func (ua *UserAPI) Post() {
 		return
 	}
 
+	if !ua.IsAdmin && !filter.ReqCarriesSession(ua.Ctx.Request) {
+		ua.SendForbiddenError(errors.New("self-registration cannot be triggered via API"))
+		return
+	}
+
 	user := models.User{}
 	if err := ua.DecodeJSONReq(&user); err != nil {
 		ua.SendBadRequestError(err)

src/core/api/user_test.go

@@ -51,7 +51,8 @@ func TestUsersPost(t *testing.T) {
 		t.Error("Error occurred while add a test User", err.Error())
 		t.Log(err)
 	} else {
-		assert.Equal(400, code, "case 1: Add user status should be 400")
+		// Should be 403 as only admin can call this API, otherwise it has to be called from browser, with session id
+		assert.Equal(http.StatusForbidden, code, "case 1: Add user status should be 400")
 	}
 
 	// case 2: register a new user with admin auth, but username is empty, expect 400

src/core/config/config.go

@@ -43,6 +43,9 @@ const (
 	defaultKeyPath                     = "/etc/core/key"
 	defaultTokenFilePath               = "/etc/core/token/tokens.properties"
 	defaultRegistryTokenPrivateKeyPath = "/etc/core/private_key.pem"
+
+	// SessionCookieName is the name of the cookie for session ID
+	SessionCookieName = "sid"
 )
 
 var (

src/core/controllers/controllers_test.go

@@ -106,10 +106,13 @@ func TestAll(t *testing.T) {
 	err := middlewares.Init()
 	assert.Nil(err)
 
+	// Has to set to dev so that the xsrf panic can be rendered as 403
+	beego.BConfig.RunMode = beego.DEV
+
 	r, _ := http.NewRequest("POST", "/c/login", nil)
 	w := httptest.NewRecorder()
 	beego.BeeApp.Handlers.ServeHTTP(w, r)
-	assert.Equal(int(401), w.Code, "'/c/login' httpStatusCode should be 401")
+	assert.Equal(http.StatusForbidden, w.Code, "'/c/login' httpStatusCode should be 403")
 
 	r, _ = http.NewRequest("GET", "/c/log_out", nil)
 	w = httptest.NewRecorder()
@@ -120,12 +123,12 @@ func TestAll(t *testing.T) {
 	r, _ = http.NewRequest("POST", "/c/reset", nil)
 	w = httptest.NewRecorder()
 	beego.BeeApp.Handlers.ServeHTTP(w, r)
-	assert.Equal(int(400), w.Code, "'/c/reset' httpStatusCode should be 400")
+	assert.Equal(http.StatusForbidden, w.Code, "'/c/reset' httpStatusCode should be 403")
 
 	r, _ = http.NewRequest("POST", "/c/userExists", nil)
 	w = httptest.NewRecorder()
 	beego.BeeApp.Handlers.ServeHTTP(w, r)
-	assert.Equal(int(500), w.Code, "'/c/userExists' httpStatusCode should be 500")
+	assert.Equal(http.StatusForbidden, w.Code, "'/c/userExists' httpStatusCode should be 403")
 
 	r, _ = http.NewRequest("GET", "/c/sendEmail", nil)
 	w = httptest.NewRecorder()

src/core/controllers/proxy.go

@@ -10,6 +10,11 @@ type RegistryProxy struct {
 	beego.Controller
 }
 
+// Prepare turn off the xsrf check for registry proxy
+func (p *RegistryProxy) Prepare() {
+	p.EnableXSRF = false
+}
+
 // Handle is the only entrypoint for incoming requests, all requests must go through this func.
 func (p *RegistryProxy) Handle() {
 	req := p.Ctx.Request

src/core/filter/sessionchecker.go

@@ -0,0 +1,30 @@
+package filter
+
+import (
+	"context"
+	beegoctx "github.com/astaxie/beego/context"
+	"github.com/goharbor/harbor/src/common/utils/log"
+	"github.com/goharbor/harbor/src/core/config"
+	"net/http"
+)
+
+// SessionReqKey is the key in the context of a request to mark the request carries session when reaching the backend
+const SessionReqKey ContextValueKey = "harbor_with_session_req"
+
+// SessionCheck is a filter to mark the requests that carries a session id, it has to be registered as
+// "beego.BeforeStatic" because beego will modify the request after execution of these filters, all requests will
+// appear to have a session id cookie.
+func SessionCheck(ctx *beegoctx.Context) {
+	req := ctx.Request
+	_, err := req.Cookie(config.SessionCookieName)
+	if err == nil {
+		ctx.Request = req.WithContext(context.WithValue(req.Context(), SessionReqKey, true))
+		log.Debug("Mark the request as no-session")
+	}
+}
+
+// ReqCarriesSession verifies if the request carries session when
+func ReqCarriesSession(req *http.Request) bool {
+	r, ok := req.Context().Value(SessionReqKey).(bool)
+	return ok && r
+}

src/core/filter/sessionchecker_test.go

@@ -0,0 +1,16 @@
+package filter
+
+import (
+	beegoctx "github.com/astaxie/beego/context"
+	"github.com/stretchr/testify/assert"
+	"net/http"
+	"testing"
+)
+
+func TestReqHasNoSession(t *testing.T) {
+	req, _ := http.NewRequest("POST", "https://127.0.0.1:8080/api/users", nil)
+	ctx := beegoctx.NewContext()
+	ctx.Request = req
+	SessionCheck(ctx)
+	assert.False(t, ReqCarriesSession(ctx.Request))
+}

src/core/main.go

@@ -161,7 +161,7 @@ func gracefulShutdown(closing, done chan struct{}) {
 
 func main() {
 	beego.BConfig.WebConfig.Session.SessionOn = true
-	beego.BConfig.WebConfig.Session.SessionName = "sid"
+	beego.BConfig.WebConfig.Session.SessionName = config.SessionCookieName
 
 	redisURL := os.Getenv("_REDIS_URL")
 	if len(redisURL) > 0 {
@@ -227,9 +227,9 @@ func main() {
 	notification.Init()
 
 	filter.Init()
+	beego.InsertFilter("/api/*", beego.BeforeStatic, filter.SessionCheck)
 	beego.InsertFilter("/*", beego.BeforeRouter, filter.SecurityFilter)
 	beego.InsertFilter("/*", beego.BeforeRouter, filter.ReadonlyFilter)
-	beego.InsertFilter("/api/*", beego.BeforeRouter, filter.MediaTypeFilter("application/json", "multipart/form-data", "application/octet-stream"))
 
 	initRouters()
 

src/core/service/notifications/admin/handler.go

@@ -46,6 +46,7 @@ type Handler struct {
 
 // Prepare ...
 func (h *Handler) Prepare() {
+	h.EnableXSRF = false
 	var data job_model.JobStatusChange
 	err := json.Unmarshal(h.Ctx.Input.CopyBody(1<<32), &data)
 	if err != nil {

src/core/service/notifications/jobs/handler.go

@@ -53,6 +53,7 @@ type Handler struct {
 
 // Prepare ...
 func (h *Handler) Prepare() {
+	h.EnableXSRF = false
 	id, err := h.GetInt64FromPath(":id")
 	if err != nil {
 		log.Errorf("Failed to get job ID, error: %v", err)

src/core/service/notifications/registry/handler.go

@@ -44,6 +44,11 @@ type NotificationHandler struct {
 const manifestPattern = `^application/vnd.docker.distribution.manifest.v\d\+(json|prettyjws)`
 const vicPrefix = "vic/"
 
+// Prepare turns off xsrf check for notification handler
+func (n *NotificationHandler) Prepare() {
+	n.EnableXSRF = false
+}
+
 // Post handles POST request, and records audit log or refreshes cache based on event.
 func (n *NotificationHandler) Post() {
 	var notification models.Notification

src/portal/lib/src/service/http-xsrf-token-extractor.service.spec.ts

@@ -0,0 +1,49 @@
+import { TestBed, inject } from '@angular/core/testing';
+
+import { HttpXsrfTokenExtractorToBeUsed } from './http-xsrf-token-extractor.service';
+import { SharedModule } from '../shared/shared.module';
+import { CookieService } from "ngx-cookie";
+
+describe('HttpXsrfTokenExtractorToBeUsed', () => {
+  let cookie =  "fdsa|ds";
+  let mockCookieService =  {
+      get: function () {
+          return cookie;
+      },
+      set: function (cookieStr: string) {
+          cookie = cookieStr;
+      }
+  };
+  beforeEach(() => {
+    TestBed.configureTestingModule({
+      imports: [
+        SharedModule
+      ],
+      providers: [
+        HttpXsrfTokenExtractorToBeUsed,
+        { provide: CookieService, useValue: mockCookieService}
+    ]
+    });
+
+  });
+
+  it('should be initialized', inject([HttpXsrfTokenExtractorToBeUsed], (service: HttpXsrfTokenExtractorToBeUsed) => {
+    expect(service).toBeTruthy();
+  }));
+
+  it('should be get right token when the cookie exists', inject([HttpXsrfTokenExtractorToBeUsed],
+    (service: HttpXsrfTokenExtractorToBeUsed) => {
+    mockCookieService.set("fdsa|ds");
+    let token = service.getToken();
+    expect(btoa(token)).toEqual(cookie.split("|")[0]);
+  }));
+
+  it('should be get right token when the cookie does not exist', inject([HttpXsrfTokenExtractorToBeUsed],
+    (service: HttpXsrfTokenExtractorToBeUsed) => {
+    mockCookieService.set(null);
+    let token = service.getToken();
+    expect(token).toBeNull();
+  }));
+
+
+});

src/portal/lib/src/service/http-xsrf-token-extractor.service.ts

@@ -0,0 +1,18 @@
+import { Injectable } from "@angular/core";
+import { HttpXsrfTokenExtractor } from "@angular/common/http";
+import { CookieService } from "ngx-cookie";
+@Injectable()
+export class HttpXsrfTokenExtractorToBeUsed extends HttpXsrfTokenExtractor {
+    constructor(
+        private cookieService: CookieService,
+    ) {
+        super();
+    }
+    public getToken(): string | null {
+        const csrfCookie = this.cookieService.get("_xsrf");
+        if (csrfCookie) {
+            return atob(csrfCookie.split("|")[0]);
+        }
+        return null;
+    }
+}

src/portal/lib/src/shared/shared.module.ts

@@ -1,6 +1,6 @@
 import { NgModule } from '@angular/core';
 import { CommonModule } from '@angular/common';
-import { HttpClientModule, HttpClient} from '@angular/common/http';
+import { HttpClientModule, HttpClientXsrfModule, HttpClient, HttpXsrfTokenExtractor } from '@angular/common/http';
 import { ClarityModule } from '@clr/angular';
 import { FormsModule, ReactiveFormsModule } from '@angular/forms';
 import { TranslateModule, TranslateLoader, MissingTranslationHandler } from '@ngx-translate/core';
@@ -12,6 +12,7 @@ import { ClipboardModule } from '../third-party/ngx-clipboard/index';
 import { MyMissingTranslationHandler } from '../i18n/missing-trans.handler';
 import { TranslatorJsonLoader } from '../i18n/local-json.loader';
 import { IServiceConfig, SERVICE_CONFIG } from '../service.config';
+import { HttpXsrfTokenExtractorToBeUsed } from '../service/http-xsrf-token-extractor.service';
 
 /*export function HttpLoaderFactory(http: Http) {
     return new TranslateHttpLoader(http, 'i18n/lang/', '-lang.json');
@@ -42,6 +43,10 @@ export function GeneralTranslatorLoader(http: HttpClient, config: IServiceConfig
     imports: [
         CommonModule,
         HttpClientModule,
+        HttpClientXsrfModule.withOptions({
+            cookieName: '_xsrf',
+            headerName: 'X-Xsrftoken'
+        }),
         FormsModule,
         ReactiveFormsModule,
         ClipboardModule,
@@ -71,6 +76,8 @@ export function GeneralTranslatorLoader(http: HttpClient, config: IServiceConfig
         MarkdownModule,
         TranslateModule,
     ],
-    providers: [CookieService]
+    providers: [
+        CookieService,
+        { provide: HttpXsrfTokenExtractor, useClass: HttpXsrfTokenExtractorToBeUsed }]
 })
 export class SharedModule { }