From e031cc6b01767296200009ba7c249db0399d474c Mon Sep 17 00:00:00 2001 From: Markus Braunbeck <32308695+mr-ks@users.noreply.github.com> Date: Sat, 11 May 2024 19:22:40 +0200 Subject: [PATCH 1/5] feat: add organization as an optional parameter in Azure DevOps connection creation. Users can now create an Azure DevOps connection using a PAT that is limited to a single organization. This means users are not required to use PATs with access to all their organizations. --- config-ui/src/api/connection/index.ts | 12 ++- config-ui/src/features/connections/utils.ts | 1 + .../components/connection-form/index.tsx | 2 + .../src/plugins/register/azure/config.tsx | 17 ++-- .../register/azure/connection-fields/index.ts | 1 + .../azure/connection-fields/organization.tsx | 89 +++++++++++++++++++ config-ui/src/types/connection.ts | 2 + 7 files changed, 116 insertions(+), 8 deletions(-) create mode 100644 config-ui/src/plugins/register/azure/connection-fields/organization.tsx diff --git a/config-ui/src/api/connection/index.ts b/config-ui/src/api/connection/index.ts index e376036c009..5a1ff5f96aa 100644 --- a/config-ui/src/api/connection/index.ts +++ b/config-ui/src/api/connection/index.ts @@ -52,6 +52,7 @@ export const test = ( | 'proxy' | 'dbUrl' | 'companyId' + | 'organization' > >, ): Promise => @@ -61,6 +62,15 @@ export const testOld = ( plugin: string, payload: Pick< IConnectionAPI, - 'endpoint' | 'authMethod' | 'username' | 'password' | 'token' | 'appId' | 'secretKey' | 'proxy' | 'dbUrl' + | 'endpoint' + | 'authMethod' + | 'username' + | 'password' + | 'token' + | 'appId' + | 'secretKey' + | 'proxy' + | 'dbUrl' + | 'organization' >, ): Promise => request(`/plugins/${plugin}/test`, { method: 'post', data: payload }); diff --git a/config-ui/src/features/connections/utils.ts b/config-ui/src/features/connections/utils.ts index 3fe4fac7524..3e6731ded99 100644 --- a/config-ui/src/features/connections/utils.ts +++ b/config-ui/src/features/connections/utils.ts @@ -40,6 +40,7 @@ export const transformConnection = (plugin: string, connection: IConnectionAPI): proxy: connection.proxy, enableGraphql: connection.enableGraphql, rateLimitPerHour: connection.rateLimitPerHour, + organization: connection.organization, }; }; diff --git a/config-ui/src/plugins/components/connection-form/index.tsx b/config-ui/src/plugins/components/connection-form/index.tsx index af285259989..6d69ebf1c09 100644 --- a/config-ui/src/plugins/components/connection-form/index.tsx +++ b/config-ui/src/plugins/components/connection-form/index.tsx @@ -73,6 +73,7 @@ export const ConnectionForm = ({ plugin, connectionId, onSuccess }: Props) => { proxy: isEqual(connection?.proxy, values.proxy) ? undefined : values.proxy, dbUrl: isEqual(connection?.dbUrl, values.dbUrl) ? undefined : values.dbUrl, companyId: isEqual(connection?.companyId, values.companyId) ? undefined : values.companyId, + organization: isEqual(connection?.appId, values.organization) ? undefined : values.organization, }) : API.connection.testOld( plugin, @@ -89,6 +90,7 @@ export const ConnectionForm = ({ plugin, connectionId, onSuccess }: Props) => { 'tenantType', 'dbUrl', 'companyId', + 'organization', ]), ), { diff --git a/config-ui/src/plugins/register/azure/config.tsx b/config-ui/src/plugins/register/azure/config.tsx index cf309875112..8a232488832 100644 --- a/config-ui/src/plugins/register/azure/config.tsx +++ b/config-ui/src/plugins/register/azure/config.tsx @@ -21,7 +21,7 @@ import { DOC_URL } from '@/release'; import { IPluginConfig } from '@/types'; import Icon from './assets/icon.svg?react'; -import { BaseURL } from './connection-fields'; +import { BaseURL, ConnectionOrganization } from './connection-fields'; export const AzureConfig: IPluginConfig = { plugin: 'azuredevops', @@ -88,13 +88,16 @@ export const AzureGoConfig: IPluginConfig = { { key: 'token', label: 'Personal Access Token', - subLabel: ( - - Learn about how to create a PAT{' '} - Please select ALL ACCESSIBLE ORGANIZATIONS for the Organization field when you create the PAT. - - ), }, + ({ initialValues, values, setValues }: any) => ( + setValues({ organization: value })} + /> + ), 'proxy', { key: 'rateLimitPerHour', diff --git a/config-ui/src/plugins/register/azure/connection-fields/index.ts b/config-ui/src/plugins/register/azure/connection-fields/index.ts index de415ebacf7..302837ff891 100644 --- a/config-ui/src/plugins/register/azure/connection-fields/index.ts +++ b/config-ui/src/plugins/register/azure/connection-fields/index.ts @@ -17,3 +17,4 @@ */ export * from './base-url'; +export * from './organization'; diff --git a/config-ui/src/plugins/register/azure/connection-fields/organization.tsx b/config-ui/src/plugins/register/azure/connection-fields/organization.tsx new file mode 100644 index 00000000000..1e2972ca382 --- /dev/null +++ b/config-ui/src/plugins/register/azure/connection-fields/organization.tsx @@ -0,0 +1,89 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +import React, { useEffect, useState } from 'react'; +import { Input, Radio, type RadioChangeEvent } from 'antd'; + +import { Block, ExternalLink } from '@/components'; +import { DOC_URL } from '@/release'; + +interface Props { + initialValue: OrganizationSettings; + value: string; + label?: string; + setValue: (value: string) => void; +} + +interface OrganizationSettings { + organization: string; + scoped: boolean; +} + +export const ConnectionOrganization = ({ label, initialValue, value, setValue }: Props) => { + const [settings, setSettings] = useState({ scoped: false, organization: '' }); + + useEffect(() => { + const org = initialValue.organization || ''; + setValue(org); + + setSettings({ organization: initialValue.organization, scoped: org !== '' }); + }, [initialValue.organization]); + + const handleChange = (e: RadioChangeEvent) => { + const scoped = e.target.value; + if (scoped) { + setValue(settings.organization); + } else { + setValue(''); + } + setSettings({ ...settings, scoped }); + }; + + const handleChangeValue = (e: React.ChangeEvent) => { + const organization = e.target.value; + setValue(organization); + setSettings({ ...settings, organization }); + }; + + return ( + <> + +

+ If you are using an organization-scoped token, please enter the organization. Otherwise make sure to create an + unscoped token.{' '} + {DOC_URL.PLUGIN.AZUREDEVOPS.AUTH_TOKEN !== '' && ( + Learn about how to create a PAT + )} +

+ + Unscoped + Scoped + + + + + + + ); +}; diff --git a/config-ui/src/types/connection.ts b/config-ui/src/types/connection.ts index b5b9573069e..6f0226c415c 100644 --- a/config-ui/src/types/connection.ts +++ b/config-ui/src/types/connection.ts @@ -31,6 +31,7 @@ export interface IConnectionAPI { companyId?: number; proxy: string; rateLimitPerHour?: number; + organization?: string; } export interface IConnectionTestResult { @@ -85,4 +86,5 @@ export interface IConnection { companyId?: number; proxy: string; rateLimitPerHour?: number; + organization?: string; } From 9ee6a7652a470384fdfc25ec506322fa64054407 Mon Sep 17 00:00:00 2001 From: Markus Braunbeck <32308695+mr-ks@users.noreply.github.com> Date: Sat, 11 May 2024 19:37:04 +0200 Subject: [PATCH 2/5] feat: add organization as an optional parameter in Azure DevOps connection creation. Users can now create an Azure DevOps connection using a PAT that is limited to a single organization. This means users are not required to use PATs with access to all their organizations. --- .../azuredevops_go/api/azuredevops/client.go | 11 +++- .../azuredevops_go/api/connection_api.go | 57 +++++++++++++------ backend/plugins/azuredevops_go/api/init.go | 2 + .../azuredevops_go/api/remote_helper.go | 24 +++++--- .../azuredevops_go/models/connection.go | 2 +- 5 files changed, 68 insertions(+), 28 deletions(-) diff --git a/backend/plugins/azuredevops_go/api/azuredevops/client.go b/backend/plugins/azuredevops_go/api/azuredevops/client.go index 36a9ef7c619..bfe4beb6d0e 100644 --- a/backend/plugins/azuredevops_go/api/azuredevops/client.go +++ b/backend/plugins/azuredevops_go/api/azuredevops/client.go @@ -65,7 +65,7 @@ func (c *Client) GetUserProfile() (Profile, errors.Error) { return Profile{}, errors.Internal.Wrap(err, "failed to read user accounts") } - if res.StatusCode == 302 || res.StatusCode == 401 { + if res.StatusCode == 203 || res.StatusCode == 401 { return Profile{}, errors.Unauthorized.New("failed to read user profile") } @@ -152,6 +152,15 @@ func (c *Client) GetProjects(args GetProjectsArgs) ([]Project, errors.Error) { if err != nil { return nil, err } + + if res.StatusCode == 203 || res.StatusCode == 401 { + return nil, errors.Unauthorized.New("failed to read projects") + } + + if res.StatusCode != 200 { + return nil, errors.Internal.New(fmt.Sprintf("failed to read projects, upstream api call failed with (%v)", res.StatusCode)) + } + err = api.UnmarshalResponse(res, &data) if err != nil { return nil, err diff --git a/backend/plugins/azuredevops_go/api/connection_api.go b/backend/plugins/azuredevops_go/api/connection_api.go index d29cac85b8d..75bc8fd47d9 100644 --- a/backend/plugins/azuredevops_go/api/connection_api.go +++ b/backend/plugins/azuredevops_go/api/connection_api.go @@ -18,6 +18,7 @@ limitations under the License. package api import ( + "context" "github.com/apache/incubator-devlake/core/errors" "github.com/apache/incubator-devlake/core/plugin" "github.com/apache/incubator-devlake/helpers/pluginhelper/api" @@ -52,17 +53,10 @@ func TestConnection(input *plugin.ApiResourceInput) (*plugin.ApiResourceOutput, }, AzuredevopsConn: conn, } - vsc := azuredevops.NewClient(&connection, nil, "https://app.vssps.visualstudio.com/") - - _, err := vsc.GetUserProfile() + body, err := testConnection(context.TODO(), connection) if err != nil { - return nil, err + return nil, plugin.WrapTestConnectionErrResp(basicRes, err) } - - body := AzuredevopsTestConnResponse{} - body.Success = true - body.Message = "success" - return &plugin.ApiResourceOutput{Body: body, Status: http.StatusOK}, nil } @@ -80,16 +74,10 @@ func TestExistingConnection(input *plugin.ApiResourceInput) (*plugin.ApiResource return nil, errors.BadInput.Wrap(err, "can't read connection from database") } - vsc := azuredevops.NewClient(connection, nil, "https://app.vssps.visualstudio.com/") - _, err = vsc.GetUserProfile() + body, err := testConnection(context.TODO(), *connection) if err != nil { - return nil, err + return nil, plugin.WrapTestConnectionErrResp(basicRes, err) } - - body := AzuredevopsTestConnResponse{} - body.Success = true - body.Message = "success" - return &plugin.ApiResourceOutput{Body: body, Status: http.StatusOK}, nil } @@ -155,3 +143,38 @@ func ListConnections(input *plugin.ApiResourceInput) (*plugin.ApiResourceOutput, func GetConnection(input *plugin.ApiResourceInput) (*plugin.ApiResourceOutput, errors.Error) { return dsHelper.ConnApi.GetDetail(input) } + +func testConnection(ctx context.Context, connection models.AzuredevopsConnection) (*AzuredevopsTestConnResponse, errors.Error) { + // validate + if vld != nil { + if err := vld.Struct(connection); err != nil { + return nil, errors.Default.Wrap(err, "error validating connection") + } + } + apiClient, err := api.NewApiClientFromConnection(ctx, basicRes, &connection) + if err != nil { + return nil, err + } + + vsc := azuredevops.NewClient(&connection, apiClient, "https://app.vssps.visualstudio.com/") + org := connection.Organization + + if org == "" { + _, err = vsc.GetUserProfile() + } else { + args := azuredevops.GetProjectsArgs{ + OrgId: org, + } + _, err = vsc.GetProjects(args) + } + if err != nil { + return nil, err + } + + connection = connection.Sanitize() + body := AzuredevopsTestConnResponse{} + body.Success = true + body.Message = "success" + + return &body, nil +} diff --git a/backend/plugins/azuredevops_go/api/init.go b/backend/plugins/azuredevops_go/api/init.go index e00bcc56c81..0401e2b3662 100644 --- a/backend/plugins/azuredevops_go/api/init.go +++ b/backend/plugins/azuredevops_go/api/init.go @@ -26,6 +26,7 @@ import ( ) var vld *validator.Validate +var basicRes context.BasicRes var dsHelper *api.DsHelper[models.AzuredevopsConnection, models.AzuredevopsRepo, models.AzuredevopsScopeConfig] var raProxy *api.DsRemoteApiProxyHelper[models.AzuredevopsConnection] @@ -34,6 +35,7 @@ var raScopeSearch *api.DsRemoteApiScopeSearchHelper[models.AzuredevopsConnection func Init(br context.BasicRes, p plugin.PluginMeta) { vld = validator.New() + basicRes = br dsHelper = api.NewDataSourceHelper[ models.AzuredevopsConnection, models.AzuredevopsRepo, diff --git a/backend/plugins/azuredevops_go/api/remote_helper.go b/backend/plugins/azuredevops_go/api/remote_helper.go index 300a3fe812a..00110daf673 100644 --- a/backend/plugins/azuredevops_go/api/remote_helper.go +++ b/backend/plugins/azuredevops_go/api/remote_helper.go @@ -58,10 +58,11 @@ func listAzuredevopsRemoteScopes( err errors.Error, ) { + org := connection.Organization vsc := azuredevops.NewClient(connection, apiClient, "https://app.vssps.visualstudio.com") if groupId == "" { - return listAzuredevopsProjects(vsc, page) + return listAzuredevopsProjects(vsc, page, org) } id := strings.Split(groupId, idSeparator) @@ -76,18 +77,23 @@ func listAzuredevopsRemoteScopes( return children, nextPage, nil } -func listAzuredevopsProjects(vsc azuredevops.Client, _ AzuredevopsRemotePagination) ( +func listAzuredevopsProjects(vsc azuredevops.Client, _ AzuredevopsRemotePagination, org string) ( children []dsmodels.DsRemoteApiScopeListEntry[models.AzuredevopsRepo], nextPage *AzuredevopsRemotePagination, err errors.Error) { - profile, err := vsc.GetUserProfile() - if err != nil { - return nil, nil, err - } - accounts, err := vsc.GetUserAccounts(profile.Id) - if err != nil { - return nil, nil, err + var accounts azuredevops.AccountResponse + if org == "" { + profile, err := vsc.GetUserProfile() + if err != nil { + return nil, nil, err + } + accounts, err = vsc.GetUserAccounts(profile.Id) + if err != nil { + return nil, nil, err + } + } else { + accounts = append(accounts, azuredevops.Account{AccountName: org}) } g, _ := errgroup.WithContext(context.Background()) diff --git a/backend/plugins/azuredevops_go/models/connection.go b/backend/plugins/azuredevops_go/models/connection.go index 53e6582ae41..3bb9f4a9614 100644 --- a/backend/plugins/azuredevops_go/models/connection.go +++ b/backend/plugins/azuredevops_go/models/connection.go @@ -52,7 +52,7 @@ func (at *AzuredevopsAccessToken) SetupAuthentication(req *http.Request) errors. type AzuredevopsConn struct { //api.RestConnection `mapstructure:",squash"` AzuredevopsAccessToken `mapstructure:",squash"` - Organization string + Organization string `json:"organization"` //Endpoint string `mapstructure:"endpoint" json:"endpoint"` Proxy string `mapstructure:"proxy" json:"proxy"` //RateLimitPerHour int `comment:"api request rate limit per hour" json:"rateLimitPerHour"` From 5b13c03d843ce74b07c884138b1c24d743586d85 Mon Sep 17 00:00:00 2001 From: Markus Braunbeck <32308695+mr-ks@users.noreply.github.com> Date: Sat, 11 May 2024 20:01:40 +0200 Subject: [PATCH 3/5] fix: adjust the test data to fix the failed unit test The test case was incorrect. When an invalid PAT is provided, the Azure DevOps API redirects the user to the login page. Go's HTTP client automatically follows the redirect when it receives a 302 status code. To circumvent this, I made adjustments to the test case. --- .../plugins/azuredevops_go/api/azuredevops/testdata/test.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/backend/plugins/azuredevops_go/api/azuredevops/testdata/test.txt b/backend/plugins/azuredevops_go/api/azuredevops/testdata/test.txt index 299bce594f2..8887e5d898a 100644 --- a/backend/plugins/azuredevops_go/api/azuredevops/testdata/test.txt +++ b/backend/plugins/azuredevops_go/api/azuredevops/testdata/test.txt @@ -13,8 +13,8 @@ Basic OnRlc3QtdG9rZW4= Content-Type: application/json; charset=utf-8; api-version=7.1 Basic OmludmFsaWQtdG9rZW4= - StatusCode: 302 - Body: Object moved + StatusCode: 203 + Body: Azure DevOps Services | Sign In Content-Type: text/html; charset=utf-8 Location: https://app.vssps.visualstudio.com/_signin From 6a4e5d06ae431d32a8a7fa156ff6b3f8bb4c40f8 Mon Sep 17 00:00:00 2001 From: Markus Braunbeck <32308695+mr-ks@users.noreply.github.com> Date: Sun, 12 May 2024 10:42:22 +0200 Subject: [PATCH 4/5] fix: TestExistingConnection now incorporates the updated values as well --- backend/plugins/azuredevops_go/api/connection_api.go | 12 ++++++++++++ .../src/plugins/components/connection-form/index.tsx | 2 +- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/backend/plugins/azuredevops_go/api/connection_api.go b/backend/plugins/azuredevops_go/api/connection_api.go index 75bc8fd47d9..1ff2bec3ac5 100644 --- a/backend/plugins/azuredevops_go/api/connection_api.go +++ b/backend/plugins/azuredevops_go/api/connection_api.go @@ -73,6 +73,18 @@ func TestExistingConnection(input *plugin.ApiResourceInput) (*plugin.ApiResource if err != nil { return nil, errors.BadInput.Wrap(err, "can't read connection from database") } + var updatedConn models.AzuredevopsConn + if err := api.Decode(input.Body, &updatedConn, nil); err != nil { + return nil, err + } + + if updatedConn.Token != "" { + connection.Token = updatedConn.Token + } + + if updatedConn.Organization != "" { + connection.Organization = updatedConn.Organization + } body, err := testConnection(context.TODO(), *connection) if err != nil { diff --git a/config-ui/src/plugins/components/connection-form/index.tsx b/config-ui/src/plugins/components/connection-form/index.tsx index 6d69ebf1c09..a0b60b0f743 100644 --- a/config-ui/src/plugins/components/connection-form/index.tsx +++ b/config-ui/src/plugins/components/connection-form/index.tsx @@ -73,7 +73,7 @@ export const ConnectionForm = ({ plugin, connectionId, onSuccess }: Props) => { proxy: isEqual(connection?.proxy, values.proxy) ? undefined : values.proxy, dbUrl: isEqual(connection?.dbUrl, values.dbUrl) ? undefined : values.dbUrl, companyId: isEqual(connection?.companyId, values.companyId) ? undefined : values.companyId, - organization: isEqual(connection?.appId, values.organization) ? undefined : values.organization, + organization: isEqual(connection?.organization, values.organization) ? undefined : values.organization, }) : API.connection.testOld( plugin, From 7f6ee55d5314a8876bea366480b4beeec2649eed Mon Sep 17 00:00:00 2001 From: Markus Braunbeck <32308695+mr-ks@users.noreply.github.com> Date: Wed, 15 May 2024 21:06:33 +0200 Subject: [PATCH 5/5] chore: replace custom merge logic --- .../plugins/azuredevops_go/api/connection_api.go | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/backend/plugins/azuredevops_go/api/connection_api.go b/backend/plugins/azuredevops_go/api/connection_api.go index 1ff2bec3ac5..7da66cd1fe5 100644 --- a/backend/plugins/azuredevops_go/api/connection_api.go +++ b/backend/plugins/azuredevops_go/api/connection_api.go @@ -69,22 +69,10 @@ func TestConnection(input *plugin.ApiResourceInput) (*plugin.ApiResourceOutput, // @Failure 500 {string} errcode.Error "Internal Error" // @Router /plugins/azuredevops/connections/{connectionId}/test [POST] func TestExistingConnection(input *plugin.ApiResourceInput) (*plugin.ApiResourceOutput, errors.Error) { - connection, err := dsHelper.ConnApi.FindByPk(input) + connection, err := dsHelper.ConnApi.GetMergedConnection(input) if err != nil { return nil, errors.BadInput.Wrap(err, "can't read connection from database") } - var updatedConn models.AzuredevopsConn - if err := api.Decode(input.Body, &updatedConn, nil); err != nil { - return nil, err - } - - if updatedConn.Token != "" { - connection.Token = updatedConn.Token - } - - if updatedConn.Organization != "" { - connection.Organization = updatedConn.Organization - } body, err := testConnection(context.TODO(), *connection) if err != nil {