Adding email fallback to Groups getByID api (#4534)

As the title suggests. 

---

#### Does this PR need a docs update or release note?

- [ ]  Yes, it's included
- [ ] 🕐 Yes, but in a later PR
- [x]  No

#### Type of change

<!--- Please check the type of change your PR introduces: --->
- [x] 🌻 Feature
- [ ] 🐛 Bugfix
- [ ] 🗺️ Documentation
- [ ] 🤖 Supportability/Tests
- [ ] 💻 CI/Deployment
- [ ] 🧹 Tech Debt/Cleanup

#### Issue(s)

<!-- Can reference multiple issues. Use one of the following "magic words" - "closes, fixes" to auto-close the Github issue. -->
* #<issue>

#### Test Plan

<!-- How will this be tested prior to merging.-->
- [x] 💪 Manual
- [x]  Unit test
- [x] 💚 E2E
This commit is contained in:
jules 2023-10-24 14:00:55 -07:00 committed by GitHub
parent a9cc29d8c6
commit 4d0be25020
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 115 additions and 7 deletions

View File

@ -28,6 +28,7 @@ const (
TestCfgSiteID = "m365siteid"
TestCfgSiteURL = "m365siteurl"
TestCfgTeamID = "m365teamid"
TestCfgTeamEmail = "m365teamemail"
TestCfgTeamSiteID = "m365teamsiteid"
TestCfgGroupID = "m365groupid"
TestCfgUserID = "m365userid"
@ -48,6 +49,7 @@ const (
EnvCorsoM365TestSiteID = "CORSO_M365_TEST_SITE_ID"
EnvCorsoM365TestSiteURL = "CORSO_M365_TEST_SITE_URL"
EnvCorsoM365TestTeamID = "CORSO_M365_TEST_TEAM_ID"
EnvCorsoM365TestTeamEmail = "CORSO_M365_TEST_TEAM_EMAIL"
EnvCorsoM365TestTeamSiteID = "CORSO_M365_TEST_TEAM_SITE_ID"
EnvCorsoSecondaryM365TestTeamID = "CORSO_SECONDARY_M365_TEST_TEAM_ID"
EnvCorsoM365TestGroupID = "CORSO_M365_TEST_GROUP_ID"
@ -209,6 +211,12 @@ func ReadTestConfig() (map[string]string, error) {
os.Getenv(EnvCorsoUnlicensedM365TestUserID),
vpr.GetString(TestCfgUnlicensedUserID),
"testevents@10rqc2.onmicrosoft.com")
fallbackTo(
testEnv,
TestCfgTeamEmail,
os.Getenv(EnvCorsoM365TestTeamEmail),
vpr.GetString(TestCfgTeamEmail),
"CorsoCITeam@10rqc2.onmicrosoft.com")
testEnv[EnvCorsoTestConfigFilePath] = os.Getenv(EnvCorsoTestConfigFilePath)
testConfig = testEnv

View File

@ -261,6 +261,18 @@ func M365TeamID(t *testing.T) string {
return strings.ToLower(cfg[TestCfgTeamID])
}
// M365TeamEmail returns a teamEmail string representing the m365TeamsEmail described
// by either the env var CORSO_M365_TEST_TEAM_EMAIL, the corso_test.toml config
// file or the default value (in that order of priority) and should belong the same group as the one
// represented by CORSO_M365_TEST_TEAM_ID. The default is a
// last-attempt fallback that will only work on alcion's testing org.
func M365TeamEmail(t *testing.T) string {
cfg, err := ReadTestConfig()
require.NoError(t, err, "retrieving m365 team email from test configuration: %+v", clues.ToCore(err))
return strings.ToLower(cfg[TestCfgTeamEmail])
}
// SecondaryM365TeamID returns a teamID string representing the secondarym365TeamID described
// by either the env var CORSO_SECONDARY_M365_TEST_TEAM_ID, the corso_test.toml config
// file or the default value (in that order of priority). The default is a

View File

@ -3,6 +3,7 @@ package api
import (
"context"
"fmt"
"net/mail"
"net/url"
"strings"
@ -95,7 +96,10 @@ func getGroups(
return results, el.Failure()
}
const filterGroupByDisplayNameQueryTmpl = "displayName eq '%s'"
const (
filterGroupByDisplayNameQueryTmpl = "displayName eq '%s'"
filterGroupByMailQueryTmpl = "proxyAddresses/any(a:a eq 'smtp:%s')"
)
// GetID can look up a group by either its canonical id (a uuid)
// or by the group's display name. If looking up the display name
@ -132,9 +136,32 @@ func (c Groups) GetByID(
return nil, graph.Stack(ctx, clues.Stack(graph.ErrResourceLocked, err))
}
logger.CtxErr(ctx, err).Info("finding group by id, falling back to display name")
logger.CtxErr(ctx, err).Info("finding group by id, falling back to secondary identifier")
}
// attempt to find by email address if the identifier looks like an email
if isEmail(identifier) {
// fall back to display name or email address
opts := &groups.GroupsRequestBuilderGetRequestConfiguration{
Headers: newEventualConsistencyHeaders(),
QueryParameters: &groups.GroupsRequestBuilderGetQueryParameters{
Filter: ptr.To(fmt.Sprintf(filterGroupByMailQueryTmpl, identifier)),
},
}
resp, err := service.Client().Groups().Get(ctx, opts)
if err != nil {
if graph.IsErrResourceLocked(err) {
err = clues.Stack(graph.ErrResourceLocked, err)
}
logger.CtxErr(ctx, err).Info("finding group by email, falling back to display name")
}
return getGroupFromResponse(ctx, resp)
}
// fall back to display name
opts := &groups.GroupsRequestBuilderGetRequestConfiguration{
Headers: newEventualConsistencyHeaders(),
QueryParameters: &groups.GroupsRequestBuilderGetQueryParameters{
@ -151,6 +178,10 @@ func (c Groups) GetByID(
return nil, graph.Wrap(ctx, err, "finding group by display name")
}
return getGroupFromResponse(ctx, resp)
}
func getGroupFromResponse(ctx context.Context, resp models.GroupCollectionResponseable) (models.Groupable, error) {
vs := resp.GetValue()
if len(vs) == 0 {
@ -159,9 +190,7 @@ func (c Groups) GetByID(
return nil, clues.Stack(graph.ErrMultipleResultsMatchIdentifier).WithClues(ctx)
}
group = vs[0]
return group, nil
return vs[0], nil
}
// GetAllSites gets all the sites that belong to a group. This is
@ -349,3 +378,8 @@ func (c Groups) GetIDAndName(
return ptr.Val(s.GetId()), ptr.Val(s.GetDisplayName()), nil
}
func isEmail(email string) bool {
_, err := mail.ParseAddress(email)
return err == nil
}

View File

@ -162,8 +162,9 @@ func (suite *GroupsIntgSuite) TestGroups_GetByID() {
defer flush()
var (
groupID = suite.its.group.id
groupsAPI = suite.its.ac.Groups()
groupID = suite.its.group.id
groupsEmail = suite.its.group.email
groupsAPI = suite.its.ac.Groups()
)
grp, err := groupsAPI.GetByID(ctx, groupID, api.CallConfig{})
@ -179,6 +180,11 @@ func (suite *GroupsIntgSuite) TestGroups_GetByID() {
id: groupID,
expectErr: assert.NoError,
},
{
name: "valid email as identifier",
id: groupsEmail,
expectErr: assert.NoError,
},
{
name: "invalid id",
id: uuid.NewString(),

View File

@ -77,6 +77,7 @@ func parseableToMap(t *testing.T, thing serialization.Parsable) map[string]any {
type ids struct {
id string
email string
driveID string
driveRootFolderID string
testContainerID string
@ -142,6 +143,7 @@ func newIntegrationTesterSetup(t *testing.T) intgTesterSetup {
// use of the TeamID is intentional here, so that we are assured
// the group has full usage of the teams api.
its.group.id = tconfig.M365TeamID(t)
its.group.email = tconfig.M365TeamEmail(t)
its.nonTeamGroup.id = tconfig.M365GroupID(t)

View File

@ -61,6 +61,32 @@ func (suite *GroupsIntgSuite) TestGroupByID() {
assert.NotEmpty(t, group.DisplayName)
}
func (suite *GroupsIntgSuite) TestGroupByID_ByEmail() {
t := suite.T()
ctx, flush := tester.NewContext(t)
defer flush()
graph.InitializeConcurrencyLimiter(ctx, true, 4)
gid := tconfig.M365TeamID(t)
group, err := m365.GroupByID(ctx, suite.acct, gid)
require.NoError(t, err, clues.ToCore(err))
require.NotNil(t, group)
assert.Equal(t, gid, group.ID, "must match expected id")
assert.NotEmpty(t, group.DisplayName)
gemail := tconfig.M365TeamEmail(t)
groupByEmail, err := m365.GroupByID(ctx, suite.acct, gemail)
require.NoError(t, err, clues.ToCore(err))
require.NotNil(t, group)
assert.Equal(t, groupByEmail, group, "must be the same group as the one gotten by id")
}
func (suite *GroupsIntgSuite) TestGroupByID_notFound() {
t := suite.T()

View File

@ -46,6 +46,8 @@ type Site struct {
// * getByID (the drive expansion doesn't work on paginated data)
// * lucky chance (not all responses contain an owner ID)
OwnerID string
// OwnerEmail may or may not contain the site owner's email.
OwnerEmail string
}
// SiteByID retrieves a specific site.
@ -120,6 +122,19 @@ func ParseSite(item models.Siteable) *Site {
item.GetDrive().GetOwner().GetUser().GetId() != nil {
s.OwnerType = SiteOwnerUser
s.OwnerID = ptr.Val(item.GetDrive().GetOwner().GetUser().GetId())
addtl := item.
GetDrive().
GetOwner().
GetUser().
GetAdditionalData()
email, err := str.AnyValueToString("email", addtl)
if err != nil {
return s
}
s.OwnerEmail = email
} else if item.GetDrive() != nil && item.GetDrive().GetOwner() != nil {
ownerItem := item.GetDrive().GetOwner()
if _, ok := ownerItem.GetAdditionalData()["group"]; ok {
@ -134,6 +149,11 @@ func ParseSite(item models.Siteable) *Site {
if err != nil {
return s
}
s.OwnerEmail, err = str.AnyValueToString("email", group)
if err != nil {
return s
}
}
}