fix: enforce id on idp creation, moving validation to validator object

fixes #391

pkg/idp/handlers.go

@@ -134,6 +134,18 @@ func (a *API) handlePartialUpdate(w http.ResponseWriter, r *http.Request) {
 
 	}
 
+	if idp.ID != ID {
+		w.WriteHeader(http.StatusBadRequest)
+		json.NewEncoder(w).Encode(
+			types.Response{
+				Message: "IDP ID in url is not matching the one in the payload",
+				Status:  http.StatusBadRequest,
+			},
+		)
+
+		return
+	}
+
 	idps, err := a.service.EditResource(r.Context(), ID, idp)
 
 	if err != nil {
@@ -190,18 +202,6 @@ func (a *API) handleCreate(w http.ResponseWriter, r *http.Request) {
 
 	}
 
-	if idp.ID != "" {
-		w.WriteHeader(http.StatusBadRequest)
-		json.NewEncoder(w).Encode(
-			types.Response{
-				Message: "IDP ID field is not allowed to be passed in",
-				Status:  http.StatusBadRequest,
-			},
-		)
-
-		return
-	}
-
 	idps, err := a.service.CreateResource(r.Context(), idp)
 
 	if err != nil {

pkg/idp/handlers_test.go

@@ -447,54 +447,6 @@ func TestHandleCreateFails(t *testing.T) {
 	}
 }
 
-func TestHandleCreateFailsIfIDPassedIn(t *testing.T) {
-	ctrl := gomock.NewController(t)
-	defer ctrl.Finish()
-
-	mockLogger := NewMockLoggerInterface(ctrl)
-	mockTracer := NewMockTracer(ctrl)
-	mockMonitor := NewMockMonitorInterface(ctrl)
-	mockService := NewMockServiceInterface(ctrl)
-
-	c := new(Configuration)
-	c.ClientSecret = "secret-9"
-	c.ID = "okta_347646e49b484037b83690b020f9f629"
-	c.ClientID = "347646e4-9b48-4037-b836-90b020f9f629"
-	c.Provider = "okta"
-	c.Mapper = "file:///etc/config/kratos/okta_schema.jsonnet"
-	c.Scope = []string{"email"}
-
-	payload, _ := json.Marshal(c)
-	req := httptest.NewRequest(http.MethodPost, "/api/v0/idps", bytes.NewReader(payload))
-
-	w := httptest.NewRecorder()
-	mux := chi.NewMux()
-	NewAPI(mockService, mockTracer, mockMonitor, mockLogger).RegisterEndpoints(mux)
-
-	mux.ServeHTTP(w, req)
-
-	res := w.Result()
-	defer res.Body.Close()
-	data, err := io.ReadAll(res.Body)
-
-	if err != nil {
-		t.Errorf("expected error to be nil got %v", err)
-	}
-
-	if res.StatusCode != http.StatusBadRequest {
-		t.Fatalf("expected HTTP status code 400 got %v", res.StatusCode)
-	}
-
-	rr := new(types.Response)
-	if err := json.Unmarshal(data, rr); err != nil {
-		t.Errorf("expected error to be nil got %v", err)
-	}
-
-	if rr.Status != http.StatusBadRequest {
-		t.Errorf("expected code to be %v got %v", http.StatusBadRequest, rr.Status)
-	}
-}
-
 func TestHandleCreateFailBadRequest(t *testing.T) {
 	ctrl := gomock.NewController(t)
 	defer ctrl.Finish()

pkg/idp/third_party.go

@@ -10,7 +10,7 @@ import "encoding/json"
 // importing the library github.com/ory/kratos fails due to compilations on their side
 type Configuration struct {
 	// ID is the provider's ID
-	ID string `json:"id" yaml:"id"`
+	ID string `json:"id" yaml:"id" validate:"required"`
 
 	// Provider is either "generic" for a generic OAuth 2.0 / OpenID Connect Provider or one of:
 	// - generic

pkg/idp/validation_test.go

@@ -104,6 +104,7 @@ func TestValidate(t *testing.T) {
 			endpoint: "",
 			body: func() []byte {
 				conf := new(Configuration)
+				conf.ID = "google_generic"
 				conf.Provider = "generic"
 				conf.IssuerURL = "mock-url"
 				conf.AuthURL = "mock-url"
@@ -124,6 +125,7 @@ func TestValidate(t *testing.T) {
 			endpoint: "/",
 			body: func() []byte {
 				conf := new(Configuration)
+				conf.ID = "microsoft_1"
 				conf.Provider = "microsoft"
 				conf.Tenant = "mock-tenant"
 				conf.SubjectSource = "me"