Refine request handle process (#9760)

* Refine request handle process

Signed-off-by: Daniel Jiang <jiangd@vmware.com>

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

@@ -42,6 +42,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

@@ -164,7 +164,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 {
@@ -244,9 +244,9 @@ func main() {
 	event.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

@@ -16,13 +16,13 @@ package admin
 
 import (
 	"encoding/json"
+	"github.com/goharbor/harbor/src/core/service/notifications"
 
 	"github.com/goharbor/harbor/src/common/dao"
 	"github.com/goharbor/harbor/src/common/job"
 	job_model "github.com/goharbor/harbor/src/common/job/models"
 	"github.com/goharbor/harbor/src/common/models"
 	"github.com/goharbor/harbor/src/common/utils/log"
-	"github.com/goharbor/harbor/src/core/api"
 	j "github.com/goharbor/harbor/src/jobservice/job"
 	"github.com/goharbor/harbor/src/pkg/scan/api/scan"
 )
@@ -39,7 +39,7 @@ var statusMap = map[string]string{
 
 // Handler handles request on /service/notifications/jobs/adminjob/*, which listens to the webhook of jobservice.
 type Handler struct {
-	api.BaseController
+	notifications.BaseHandler
 	id            int64
 	UUID          string
 	status        string
@@ -52,6 +52,7 @@ type Handler struct {
 
 // Prepare ...
 func (h *Handler) Prepare() {
+	h.BaseHandler.Prepare()
 	var data job_model.JobStatusChange
 	err := json.Unmarshal(h.Ctx.Input.CopyBody(1<<32), &data)
 	if err != nil {

src/core/service/notifications/base.go

@@ -0,0 +1,13 @@
+package notifications
+
+import "github.com/goharbor/harbor/src/core/api"
+
+// BaseHandler extracts the common funcs, all notification handlers should shadow this struct
+type BaseHandler struct {
+	api.BaseController
+}
+
+// Prepare disable the xsrf as the request is from other components and do not require the xsrf token
+func (bh *BaseHandler) Prepare() {
+	bh.EnableXSRF = false
+}

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

@@ -16,12 +16,12 @@ package jobs
 
 import (
 	"encoding/json"
+	"github.com/goharbor/harbor/src/core/service/notifications"
 	"time"
 
 	"github.com/goharbor/harbor/src/common/job"
 	"github.com/goharbor/harbor/src/common/models"
 	"github.com/goharbor/harbor/src/common/utils/log"
-	"github.com/goharbor/harbor/src/core/api"
 	"github.com/goharbor/harbor/src/core/notifier/event"
 	jjob "github.com/goharbor/harbor/src/jobservice/job"
 	"github.com/goharbor/harbor/src/pkg/notification"
@@ -45,7 +45,7 @@ var statusMap = map[string]string{
 
 // Handler handles request on /service/notifications/jobs/*, which listens to the webhook of jobservice.
 type Handler struct {
-	api.BaseController
+	notifications.BaseHandler
 	id        int64
 	status    string
 	rawStatus string
@@ -57,6 +57,7 @@ type Handler struct {
 
 // Prepare ...
 func (h *Handler) Prepare() {
+	h.BaseHandler.Prepare()
 	h.trackID = h.GetStringFromPath(":uuid")
 	if len(h.trackID) == 0 {
 		id, err := h.GetInt64FromPath(":id")

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

@@ -16,6 +16,7 @@ package registry
 
 import (
 	"encoding/json"
+	"github.com/goharbor/harbor/src/core/service/notifications"
 	"regexp"
 	"strconv"
 	"strings"
@@ -25,7 +26,6 @@ import (
 	"github.com/goharbor/harbor/src/common/models"
 	"github.com/goharbor/harbor/src/common/utils"
 	"github.com/goharbor/harbor/src/common/utils/log"
-	"github.com/goharbor/harbor/src/core/api"
 	"github.com/goharbor/harbor/src/core/config"
 	notifierEvt "github.com/goharbor/harbor/src/core/notifier/event"
 	coreutils "github.com/goharbor/harbor/src/core/utils"
@@ -41,7 +41,7 @@ import (
 
 // NotificationHandler handles request on /service/notifications/, which listens to registry's events.
 type NotificationHandler struct {
-	api.BaseController
+	notifications.BaseHandler
 }
 
 const manifestPattern = `^application/vnd.docker.distribution.manifest.v\d\+(json|prettyjws)`

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

@@ -17,17 +17,17 @@ package scheduler
 import (
 	"encoding/json"
 	"fmt"
+	"github.com/goharbor/harbor/src/core/service/notifications"
 
 	"github.com/goharbor/harbor/src/common/job/models"
 	"github.com/goharbor/harbor/src/common/utils/log"
-	"github.com/goharbor/harbor/src/core/api"
 	"github.com/goharbor/harbor/src/pkg/scheduler"
 	"github.com/goharbor/harbor/src/pkg/scheduler/hook"
 )
 
 // Handler handles the scheduler requests
 type Handler struct {
-	api.BaseController
+	notifications.BaseHandler
 }
 
 // Handle ...

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 { }

src/portal/src/app/dev-center/dev-center.component.spec.ts

@@ -2,11 +2,16 @@ import { async, ComponentFixture, TestBed } from '@angular/core/testing';
 import { HttpClientTestingModule } from '@angular/common/http/testing';
 import { TranslateModule, TranslateService } from '@ngx-translate/core';
 import { DevCenterComponent } from './dev-center.component';
+import { CookieService } from 'ngx-cookie';
 
 describe('DevCenterComponent', () => {
   let component: DevCenterComponent;
   let fixture: ComponentFixture<DevCenterComponent>;
-
+  const mockCookieService = {
+    get: () => {
+      return "xsrf";
+    }
+  };
   beforeEach(async(() => {
     TestBed.configureTestingModule({
       declarations: [DevCenterComponent],
@@ -15,7 +20,10 @@ describe('DevCenterComponent', () => {
         TranslateModule.forRoot()
       ],
       providers: [
-        TranslateService
+        TranslateService,
+        {
+          provide: CookieService, useValue: mockCookieService
+        }
       ],
     })
       .compileComponents();

src/portal/src/app/dev-center/dev-center.component.ts

@@ -4,7 +4,7 @@ import { throwError as observableThrowError, Observable } from 'rxjs';
 import { catchError, map } from 'rxjs/operators';
 import { Title } from '@angular/platform-browser';
 import { TranslateService } from '@ngx-translate/core';
-
+import { CookieService } from "ngx-cookie";
 
 const SwaggerUI = require('swagger-ui');
 @Component({
@@ -21,6 +21,7 @@ export class DevCenterComponent implements AfterViewInit, OnInit {
     private el: ElementRef,
     private http: HttpClient,
     private translate: TranslateService,
+    private cookieService: CookieService,
     private titleService: Title) {
   }
 
@@ -28,28 +29,47 @@ export class DevCenterComponent implements AfterViewInit, OnInit {
     this.setTitle("APP_TITLE.HARBOR_SWAGGER");
   }
 
-
-  public setTitle( key: string) {
+  public setTitle(key: string) {
     this.translate.get(key).subscribe((res: string) => {
       this.titleService.setTitle(res);
-  });
+    });
   }
 
   ngAfterViewInit() {
+    const csrfCookie = this.cookieService.get('_xsrf');
+    const interceptor = {
+      requestInterceptor: {
+        apply: function (requestObj) {
+          const headers = requestObj.headers || {};
+          if (csrfCookie) {
+            headers["X-Xsrftoken"] = atob(csrfCookie.split("|")[0]);
+          }
+          return requestObj;
+        }
+      }
+    };
     this.http.get("/swagger.json")
-    .pipe(catchError(error => observableThrowError(error)))
-    .subscribe(json => {
-      json['host'] = window.location.host;
-      const protocal = window.location.protocol;
-      json['schemes'] = [protocal.replace(":", "")];
-      let ui = SwaggerUI({
-        spec: json,
-        domNode: this.el.nativeElement.querySelector('.swagger-container'),
-        deepLinking: true,
-        presets: [
-          SwaggerUI.presets.apis
-        ],
+      .pipe(catchError(error => observableThrowError(error)))
+      .subscribe(json => {
+        json['host'] = window.location.host;
+        const protocal = window.location.protocol;
+        json['schemes'] = [protocal.replace(":", "")];
+        let ui = SwaggerUI({
+          spec: json,
+          domNode: this.el.nativeElement.querySelector('.swagger-container'),
+          deepLinking: true,
+          presets: [
+            SwaggerUI.presets.apis
+          ],
+          requestInterceptor: interceptor.requestInterceptor,
+          authorizations: {
+            csrf: function () {
+              this.headers['X-Xsrftoken'] = csrfCookie;
+              return true;
+            }
+          }
+        });
       });
-    });
   }
+
 }