Univention Bugzilla – Bug 52840
Encapsulate Graph API functionality to work with Microsoft 365 Teams
Last modified: 2021-09-13 17:23:16 CEST
We need the backend functionality to work with Microsoft teams. For that we have to log in via Microsoft Graph API and then call team related API functions. We want to create teams, edit teams, delete teams and create teams out of already existing groups. Only groups with an owner can be converted to a team. That makes it necessary to implement a function to add owners to existing groups before converting groups without owners. # Original title Encapsulate library functions in Office 365 ## Original acceptance criteria * The backend functionality to create teams groups via cli/python-shell is encapsulated as library functions in the Office 365 connector lib * We have automatic tests for the library functions. * Login, Create / Edit groups (add users), delete (class CRUD operations)
Implemented with 2.0.2-101A~4.4.7.202103151531 and commit c7b4535 QA please.
Please QA 2.0.2-101A~4.4.0.202103151545
this break the listener 15.03.21 17:32:41.323 LISTENER ( ERROR ) : import of filename=/usr/lib/univention-directory-listener/system/office365-user.py failed Traceback (most recent call last): File "/usr/lib/univention-directory-listener/system/office365-user.py", line 180, in <module> AzureAuth.get_http_proxies() # log proxy settings AttributeError: type object 'AzureAuth' has no attribute 'get_http_proxies'
fixed in 2.0.2-102A~4.4.0.202103160920
/usr/share/ucs-test/92_office365/303_add_user_to_group_twice fails UCSTestUDM cleanup done Failed to open /dev/tty: Kein passendes Gerät bzw. keine passende Adresse gefunden Restarting univention-directory-listener (via systemctl): univention-directory-listener.service. Traceback (most recent call last): File "303_add_user_to_group_twice", line 93, in <module> ol.ah.add_objects_to_azure_group(group_objectid, [user_id1]) File "/usr/lib/pymodules/python2.7/univention/office365/azure_handler.py", line 562, in add_objects_to_azure_group self.call_api("POST", url, data=objs) File "/usr/lib/pymodules/python2.7/univention/office365/azure_handler.py", line 300, in call_api raise ApiError(response, adconnection_alias=self.adconnection_alias) univention.office365.azure_handler.ApiError: One or more added object references already exist for the following modified properties: 'members'.> request url: https://graph.windows.net/d66c72f6-8dee-47ed-ac2b-7fdbe8b8a5d4/groups/60e2e629-4938-48c3-8ad2-49444363c453/$links/members?api-version=1.6 > request header: { "Content-Length": "127", "Accept-Encoding": "gzip, deflate", "Accept": "application/json", "User-Agent": "ucs-office365/1.0", "return-client-request-id": "true", "Connection": "keep-alive", "client-request-id": "a95d7b1a-cb68-46ca-aa16-d54ea9229e34", "Content-Type": "application/json", "Authorization": "Bearer eyJ0eXAiOiJKV1QiLCJhbGciOiJSUzI1NiIsIng1dCI6Im5PbzNaRHJPRFhFSzFqS1doWHNsSFJfS1hFZyIsImtpZCI6Im5PbzNaRHJPRFhFSzFqS1doWHNsSFJfS1hFZyJ9.eyJhdWQiOiJodHRwczovL2dyYXBoLndpbmRvd3MubmV0IiwiaXNzIjoiaHR0cHM6Ly9zdHMud2luZG93cy5uZXQvZDY2YzcyZjYtOGRlZS00N2VkLWFjMmItN2ZkYmU4YjhhNWQ0LyIsImlhdCI6MTYxNTg5MDIyMywibmJmIjoxNjE1ODkwMjIzLCJleHAiOjE2MTU4OTQxMjMsImFpbyI6IkUyWmdZTGhSNnZhNzlmeER2dW5UWjdGOWJ2NVRDUUE9IiwiYXBwaWQiOiIyY2M1NzkxOC0zMGQ3LTQ3ZGYtOWFlZi00ZmM0YTMwYWIwZjIiLCJhcHBpZGFjciI6IjIiLCJpZHAiOiJodHRwczovL3N0cy53aW5kb3dzLm5ldC9kNjZjNzJmNi04ZGVlLTQ3ZWQtYWMyYi03ZmRiZThiOGE1ZDQvIiwib2lkIjoiNzNjNzMyMDUtNDdkMS00MzgzLTkzNmItMGU5YTVhNmNiNmE1IiwicmgiOiIwLkFUd0E5bkpzMXU2TjdVZXNLM19iNkxpbDFCaDV4U3pYTU45SG11OVB4S01Lc1BJOEFBQS4iLCJyb2xlcyI6WyJEaXJlY3RvcnkuUmVhZFdyaXRlLkFsbCJdLCJzdWIiOiI3M2M3MzIwNS00N2QxLTQzODMtOTM2Yi0wZTlhNWE2Y2I2YTUiLCJ0ZW5hbnRfcmVnaW9uX3Njb3BlIjoiRVUiLCJ0aWQiOiJkNjZjNzJmNi04ZGVlLTQ3ZWQtYWMyYi03ZmRiZThiOGE1ZDQiLCJ1dGkiOiI0emRJR2U2UTRVZXRuSmlrY3Z0RkFBIiwidmVyIjoiMS4wIn0.UOiFRB2nPOAEUcX3vPRpbS-XdJUlJYsB63fTjfabh6hcKbsfTy0UFzjqmUGpd8A4Y0SYMiT29b1eHcZVC_yX8t8x-5OYWauSyMblm7-bkYOrpqZ3V3IIVS0fLiXZq2ETAhG8hye_l2of5ijt3HfnqLLBmAYnu217AaRuGZsedMSyR7YHDDvVEwDceznrQE0yP1ew_sPIHXo4D2vGVmLC7y809dcB-9w3gzHEZxdcAwzRmYHsSm16_HfWTWRE-Ckugfhmk4Z7Pj1McFGOuSNn2-RTwHhpgrLc5hFvfD9mhkjP_94NXIeu8jdx_rabMLuso-k1QLA4hh0_LWD6Op4vvg" } > request body: { "url": "https://graph.windows.net/d66c72f6-8dee-47ed-ac2b-7fdbe8b8a5d4/directoryObjects/2a71ce34-b2ca-4725-87d4-151614a3a8ac" } > response header: { "Content-Length": "259", "Expires": "-1", "DataServiceVersion": "3.0;", "X-AspNet-Version": "4.0.30319", "request-id": "4e2e127f-8cf0-4976-901d-fc5caf7f2a07", "ocp-aad-diagnostics-server-name": "BSti9UM88FZrs42+Nma/qk84nZBqrT5vle2AcnRbNgI=", "X-Powered-By": "ASP.NET", "ocp-aad-session-key": "KzXoLZSnZoGplTZO32eo0crRJH0-CIw8lSvpcMLk2pCnsBnpMSxmZmRFj-r1wdhgO64MEHeW-QzOb_Tem4xo9yc1aFD9ZaDO73MwGy_9RvTtxnjowrw26_foxOp_TZecKHK4aeJ6UQ6eTO2fDDIssSpwym9GOznA9Qo1qAaCScQ.tzeNJpMotWujTMOXTH0SBnwyUifZur3XJUHiWgauLpI", "Date": "Tue, 16 Mar 2021 10:29:38 GMT", "x-ms-resource-unit": "1", "client-request-id": "a95d7b1a-cb68-46ca-aa16-d54ea9229e34", "Pragma": "no-cache", "Cache-Control": "no-cache", "Duration": "1015747", "Strict-Transport-Security": "max-age=31536000; includeSubDomains", "Content-Type": "application/json; odata=minimalmetadata; streaming=true; charset=utf-8", "Access-Control-Allow-Origin": "*", "x-ms-dirapi-data-contract-version": "1.6" } > response body: { "odata.error": { "date": "2021-03-16T10:29:39", "message": { "lang": "en", "value": "One or more added object references already exist for the following modified properties: 'members'." }, "code": "Request_BadRequest", "requestId": "4e2e127f-8cf0-4976-901d-fc5caf7f2a07" } } Starting 1 ucs-test at 2021-03-16 11:29:49 to /dev/null UCS 4.4-7-e910 ucs-test 9.0.7-25A~4.4.0.202103021459 create user in azure and add to group twice.................................................................................................................................... Test failed don't know why, but it seems to be this change --- a/modules/univention/office365/azure_handler.py +++ b/modules/univention/office365/azure_handler.py @@ -140,16 +140,39 @@ def get_service_plan_names(ucr): class ApiError(AzureError): def __init__(self, response, *args, **kwargs): msg = "Communication error." + if isinstance(response, requests.Response): + msg += "HTTP response status: {num}\n".format( + num=response.status_code + ) if hasattr(response, "json"): j = response.json if callable(j): # requests version compatibility j = j() msg = j["odata.error"]["message"]["value"] self.json = j + msg += ( + "> request url: {req_url}\n\n" + "> request header: {req_headers}\n\n" + "> request body: {req_body}\n\n" + "> response header: {headers}\n\n" + "> response body: {body}\n\n" + ).format( + req_url=str(response.request.url), + req_headers=json.dumps(dict(response.request.headers), indent=2), + req_body=self._try_to_prettify(response.request.body or "-NONE-"), + headers=json.dumps(dict(response.headers), indent=2), + body=self._try_to_prettify(response.content or "-NONE-") + ) self.response = response logger.error(msg) super(ApiError, self).__init__(msg, *args, **kwargs) if this is reverted, the tests succeeds
(In reply to Felix Botner from comment #5) > /usr/share/ucs-test/92_office365/303_add_user_to_group_twice fails > > UCSTestUDM cleanup done > Failed to open /dev/tty: Kein passendes Gerät bzw. keine passende Adresse > gefunden > Restarting univention-directory-listener (via systemctl): > univention-directory-listener.service. > Traceback (most recent call last): > File "303_add_user_to_group_twice", line 93, in <module> > ol.ah.add_objects_to_azure_group(group_objectid, [user_id1]) > File "/usr/lib/pymodules/python2.7/univention/office365/azure_handler.py", > line 562, in add_objects_to_azure_group > self.call_api("POST", url, data=objs) > File "/usr/lib/pymodules/python2.7/univention/office365/azure_handler.py", > line 300, in call_api > raise ApiError(response, adconnection_alias=self.adconnection_alias) > univention.office365.azure_handler.ApiError: One or more added object > references already exist for the following modified properties: 'members'.> > request url: > https://graph.windows.net/d66c72f6-8dee-47ed-ac2b-7fdbe8b8a5d4/groups/ > 60e2e629-4938-48c3-8ad2-49444363c453/$links/members?api-version=1.6 > > > request header: { > "Content-Length": "127", > "Accept-Encoding": "gzip, deflate", > "Accept": "application/json", > "User-Agent": "ucs-office365/1.0", > "return-client-request-id": "true", > "Connection": "keep-alive", > "client-request-id": "a95d7b1a-cb68-46ca-aa16-d54ea9229e34", > "Content-Type": "application/json", > "Authorization": "Bearer > eyJ0eXAiOiJKV1QiLCJhbGciOiJSUzI1NiIsIng1dCI6Im5PbzNaRHJPRFhFSzFqS1doWHNsSFJfS > 1hFZyIsImtpZCI6Im5PbzNaRHJPRFhFSzFqS1doWHNsSFJfS1hFZyJ9. > eyJhdWQiOiJodHRwczovL2dyYXBoLndpbmRvd3MubmV0IiwiaXNzIjoiaHR0cHM6Ly9zdHMud2luZ > G93cy5uZXQvZDY2YzcyZjYtOGRlZS00N2VkLWFjMmItN2ZkYmU4YjhhNWQ0LyIsImlhdCI6MTYxNT > g5MDIyMywibmJmIjoxNjE1ODkwMjIzLCJleHAiOjE2MTU4OTQxMjMsImFpbyI6IkUyWmdZTGhSNnZ > hNzlmeER2dW5UWjdGOWJ2NVRDUUE9IiwiYXBwaWQiOiIyY2M1NzkxOC0zMGQ3LTQ3ZGYtOWFlZi00 > ZmM0YTMwYWIwZjIiLCJhcHBpZGFjciI6IjIiLCJpZHAiOiJodHRwczovL3N0cy53aW5kb3dzLm5ld > C9kNjZjNzJmNi04ZGVlLTQ3ZWQtYWMyYi03ZmRiZThiOGE1ZDQvIiwib2lkIjoiNzNjNzMyMDUtND > dkMS00MzgzLTkzNmItMGU5YTVhNmNiNmE1IiwicmgiOiIwLkFUd0E5bkpzMXU2TjdVZXNLM19iNkx > pbDFCaDV4U3pYTU45SG11OVB4S01Lc1BJOEFBQS4iLCJyb2xlcyI6WyJEaXJlY3RvcnkuUmVhZFdy > aXRlLkFsbCJdLCJzdWIiOiI3M2M3MzIwNS00N2QxLTQzODMtOTM2Yi0wZTlhNWE2Y2I2YTUiLCJ0Z > W5hbnRfcmVnaW9uX3Njb3BlIjoiRVUiLCJ0aWQiOiJkNjZjNzJmNi04ZGVlLTQ3ZWQtYWMyYi03Zm > RiZThiOGE1ZDQiLCJ1dGkiOiI0emRJR2U2UTRVZXRuSmlrY3Z0RkFBIiwidmVyIjoiMS4wIn0. > UOiFRB2nPOAEUcX3vPRpbS- > XdJUlJYsB63fTjfabh6hcKbsfTy0UFzjqmUGpd8A4Y0SYMiT29b1eHcZVC_yX8t8x- > 5OYWauSyMblm7- > bkYOrpqZ3V3IIVS0fLiXZq2ETAhG8hye_l2of5ijt3HfnqLLBmAYnu217AaRuGZsedMSyR7YHDDvV > EwDceznrQE0yP1ew_sPIHXo4D2vGVmLC7y809dcB-9w3gzHEZxdcAwzRmYHsSm16_HfWTWRE- > Ckugfhmk4Z7Pj1McFGOuSNn2-RTwHhpgrLc5hFvfD9mhkjP_94NXIeu8jdx_rabMLuso- > k1QLA4hh0_LWD6Op4vvg" > } > > > request body: { > "url": > "https://graph.windows.net/d66c72f6-8dee-47ed-ac2b-7fdbe8b8a5d4/ > directoryObjects/2a71ce34-b2ca-4725-87d4-151614a3a8ac" > } > > > response header: { > "Content-Length": "259", > "Expires": "-1", > "DataServiceVersion": "3.0;", > "X-AspNet-Version": "4.0.30319", > "request-id": "4e2e127f-8cf0-4976-901d-fc5caf7f2a07", > "ocp-aad-diagnostics-server-name": > "BSti9UM88FZrs42+Nma/qk84nZBqrT5vle2AcnRbNgI=", > "X-Powered-By": "ASP.NET", > "ocp-aad-session-key": > "KzXoLZSnZoGplTZO32eo0crRJH0-CIw8lSvpcMLk2pCnsBnpMSxmZmRFj-r1wdhgO64MEHeW- > QzOb_Tem4xo9yc1aFD9ZaDO73MwGy_9RvTtxnjowrw26_foxOp_TZecKHK4aeJ6UQ6eTO2fDDIssS > pwym9GOznA9Qo1qAaCScQ.tzeNJpMotWujTMOXTH0SBnwyUifZur3XJUHiWgauLpI", > "Date": "Tue, 16 Mar 2021 10:29:38 GMT", > "x-ms-resource-unit": "1", > "client-request-id": "a95d7b1a-cb68-46ca-aa16-d54ea9229e34", > "Pragma": "no-cache", > "Cache-Control": "no-cache", > "Duration": "1015747", > "Strict-Transport-Security": "max-age=31536000; includeSubDomains", > "Content-Type": "application/json; odata=minimalmetadata; streaming=true; > charset=utf-8", > "Access-Control-Allow-Origin": "*", > "x-ms-dirapi-data-contract-version": "1.6" > } > > > response body: { > "odata.error": { > "date": "2021-03-16T10:29:39", > "message": { > "lang": "en", > "value": "One or more added object references already exist for the > following modified properties: 'members'." > }, > "code": "Request_BadRequest", > "requestId": "4e2e127f-8cf0-4976-901d-fc5caf7f2a07" > } > } > > > Starting 1 ucs-test at 2021-03-16 11:29:49 to /dev/null > UCS 4.4-7-e910 ucs-test 9.0.7-25A~4.4.0.202103021459 > create user in azure and add to group > twice........................................................................ > ............................................................ Test failed > > don't know why, but it seems to be this change > --- a/modules/univention/office365/azure_handler.py > +++ b/modules/univention/office365/azure_handler.py > @@ -140,16 +140,39 @@ def get_service_plan_names(ucr): > class ApiError(AzureError): > def __init__(self, response, *args, **kwargs): > msg = "Communication error." > + if isinstance(response, requests.Response): > + msg += "HTTP response status: {num}\n".format( > + num=response.status_code > + ) > if hasattr(response, "json"): > j = response.json > if callable(j): # requests version compatibility > j = j() > msg = j["odata.error"]["message"]["value"] > self.json = j > + msg += ( > + "> request url: {req_url}\n\n" > + "> request header: {req_headers}\n\n" > + "> request body: {req_body}\n\n" > + "> response header: {headers}\n\n" > + "> response body: {body}\n\n" > + ).format( > + req_url=str(response.request.url), > + > req_headers=json.dumps(dict(response.request.headers), indent=2), > + > req_body=self._try_to_prettify(response.request.body or "-NONE-"), > + headers=json.dumps(dict(response.headers), > indent=2), > + body=self._try_to_prettify(response.content > or "-NONE-") > + ) > self.response = response > logger.error(msg) > super(ApiError, self).__init__(msg, *args, **kwargs) > > > if this is reverted, the tests succeeds that is because of this except ApiError as exc: # ignore error if object is already member of group if str(exc) == "One or more added object references already exist for the following modified properties: 'members'.": logger.info("Ignore ApiError 'One or more added object references already exist ...' in add_objects_to_azure_group, object is already member of group") else: raise Currently we ignore these errors "One or more added object references already exist for the following modified properties: 'members'." ... azure_handler.call_api:254 POST https://graph.windows.net/d66c72f6-8dee-47ed-ac2b-7fdbe8b8a5d4/groups/2ebdb855-39b3-472d-8500-75ace9feb7c6/$links/members?api-version=1.6 data: {'url': 'https://graph.windows.net/d66c72f6-8dee-47ed-ac2b-7fdbe8b8a5d4/directoryObjects/60c18378-1128-48a8-bd45-ff12a0ac9712'} status: 400 (FAIL) Code: Request_BadRequest (POST https://graph.windows.net/d66c72f6-8dee-47ed-ac2b-7fdbe8b8a5d4/groups/2ebdb855-39b3-472d-8500-75ace9feb7c6/$links/members?api-version=1.6) One or more added object references already exist for the following modified properties: 'members'. Ignore ApiError 'One or more added object references already exist ...' in add_objects_to_azure_group, object is already member of group ... But now we have changed that message
fixed in 2.0.2-103A~4.4.0.202103161421
402_teams_test seems to have a problem ... /usr/share/ucs-test/92_office365/402_teams_test -f Traceback (most recent call last): File "402_teams_test", line 20, in <module> from univention.office365.api.graph_auth import get_all_aliases_from_ucr ImportError: No module named graph_auth Starting 1 ucs-test at 2021-03-16 14:39:38 to /dev/null UCS 4.4-7-e910 ucs-test 9.0.7-25A~4.4.0.202103021459 tests the team related API calls, return values and exception handling......................................................................................................... Test failed
and 401_graph_backward_compatibility_test Traceback (most recent call last): File "401_graph_backward_compatibility_test", line 17, in <module> from univention.office365.api.graph_auth import get_all_aliases_from_ucr ImportError: No module named graph_auth Starting 1 ucs-test at 2021-03-16 14:41:07 to /dev/null What is the idea behind 401_graph_backward_compatibility_test?
Please rename load_token_file to load_ids (or something like that) and remove token.json from that function (token.json is the access token cache for the old "api").
(1) Not sure about that list_teams() function. Is that not just list groups? Don't we need something that "loads" all groups and return only those with {u'resourceProvisioningOptions': [u'Team']} ? https://docs.microsoft.com/en-us/graph/teams-list-all-teams?view=graph-rest-1.0 And why is there a page_size=500 option for this particular function (only). Either remove this option completely (paging should work anyway, or?) or move this option to call_graph_api (configuration iia UCR?). (2) I have a test setup with wrong time settings, a successful graph login is not possible therefor. But starting the graph api results in an endless loop. i guess this is the reason def _call_graph_api ... elif 401 == response.status_code: self._login(self.connection_alias) continue and _login itself uses _call_graph_api, we have to fix this. The logic should be, try the access token from the file if that fails, get a new access token, if that fails throw an error.
Hey Felix! I do not have enough time to comment on everything you mentioned right now and will come back to that later. For now please use version `2.0.2-105A~4.4.0.202103171117` for further testing.
Hi again, Felix! In comment 5 you said: > if this is reverted, the tests succeeds It is true, that the new code made the test fail. That was, because the test expected a certain message as a return value from the error and was not able to parse multi-line-error messages it seems. I still consider the back-port of the error message from Graph to Azure a good idea, because not having such meaningful error messages have slowed me down when I had my first contact with the class. The current implementation uses the logger.debug to display the message only at the highest debug level. The msg from that snippet remains untouched and that fixes the test as well and applies also to comment 6. The fixed was introduced with 2.0.2-103A~4.4.0.202103161421 In comment 8 you meantioned the error > Traceback (most recent call last): > File "402_teams_test", line 20, in <module> > from univention.office365.api.graph_auth import get_all_aliases_from_ucr > ImportError: No module named graph_auth This error was due to a renamed function, which does not rely on the values from ucr any more and was thus renamed to get_all_aliases. The function was at some point moved to the `terminaltest.py` program, because it was believed, that it is only used there. As it turns out: The tests also used it and the function was made a library function again (with this new name). The reason for removing the binding to the ucr is, that it simplifies the test setup. Copy `/etc/univention-office365` and you are done. That improves the usability of `terminaltest.py`, which displays a list of aliases in its --help and has thrown an error without the necessary ucr variables. It now works independently and thus more stable. In comment 9 you asked: > What is the idea behind 401_graph_backward_compatibility_test? The graph.py has the Graph class, which is based on AzureHandler from azure-handler.py. It supports all function of the AzureHandler and adds team related functions to that. The Graph class can be used as a drop-in-replacement where the AzureHandler class was previously used. During the development we decided together, that a `call_api` function is required similar to the one found in the AzureHandler class. Now imagine you would name a function like that in Graph: It would then override the function from the base class. Nobody would notice that as long as the new call_api function from graph.py behaves in exactly the same way as the call_api function from the azure_handler.py did. This test was prepared to ensure just that. In its current state the test creates group using the AzureHandler directly and indirectly via the Graph class and if both return the same result, the test is successfull. This only works, if the Graph class was able to initialize the access token for the AzureHandler class correctly, which makes this test even more meaningful. In comment 10 you wrote: > Please rename load_token_file to load_ids [...] done. In comment 11 you wonderd: > Not sure about that list_teams() function. [...] > Is that not just list groups? Yes it is and it is the way how it is documented on the Microsoft site. I had the choice here to use the new beta API to get the list of teams as it is described here: https://docs.microsoft.com/en-us/graph/teams-list-all-teams#get-a-list-of-groups-using-beta-apis The beta API would possibly perform better, but for now I decided against it, because the beta API could be more unstable. I could have used the list_groups functions from the base class at this point, but I would consider that an unreasonable implementation decision as well, because it would introduce a stronger binding between the two classes. It would also be inconsistent with other calls within the Graph class in that it would then use a different access token for the call. This makes the current implementation more stable and future proof in the way, that we can still modify code in the AzureHandler class as needed without having to worry about compatibility to Graph. Also in Comment 11 you said: > I have a test setup with wrong time settings, a successful graph login is not possible therefor. But starting the graph api results in an endless loop. Good point. That problem came from me starting to use call_graph_api for logins as well. That will be fixed with the next version. I can still fix tests without addressing this issue and the fix is really simple. I will postpone it for now, it will be in the next package and I will keep you up-to-date.
(In reply to Max Pohle from comment #13) > Hi again, Felix! > > In comment 5 you said: > > if this is reverted, the tests succeeds > > It is true, that the new code made the test fail. That was, because the test > expected a certain message as a return value from the error and was not able > to parse multi-line-error messages it seems. I still consider the back-port > of the error message from Graph to Azure a good idea, because not having > such meaningful error messages have slowed me down when I had my first > contact with the class. The current implementation uses the logger.debug to > display the message only at the highest debug level. The msg from that > snippet remains untouched and that fixes the test as well and applies also > to comment 6. The fixed was introduced with 2.0.2-103A~4.4.0.202103161421 OK > In comment 8 you meantioned the error > > Traceback (most recent call last): > > File "402_teams_test", line 20, in <module> > > from univention.office365.api.graph_auth import get_all_aliases_from_ucr > > ImportError: No module named graph_auth > > > This error was due to a renamed function, which does not rely on the values > from ucr any more and was thus renamed to get_all_aliases. The function was > at some point moved to the `terminaltest.py` program, because it was > believed, that it is only used there. As it turns out: The tests also used > it and the function was made a library function again (with this new name). > The reason for removing the binding to the ucr is, that it simplifies the > test setup. Copy `/etc/univention-office365` and you are done. That improves > the usability of `terminaltest.py`, which displays a list of aliases in its > --help and has thrown an error without the necessary ucr variables. It now > works independently and thus more stable. OK > > In comment 9 you asked: > > > What is the idea behind 401_graph_backward_compatibility_test? > > The graph.py has the Graph class, which is based on AzureHandler from > azure-handler.py. It supports all function of the AzureHandler and adds team > related functions to that. The Graph class can be used as a > drop-in-replacement where the AzureHandler class was previously used. > > During the development we decided together, that a `call_api` function is > required similar to the one found in the AzureHandler class. Now imagine you > would name a function like that in Graph: It would then override the > function from the base class. Nobody would notice that as long as the new > call_api function from graph.py behaves in exactly the same way as the > call_api function from the azure_handler.py did. This test was prepared to > ensure just that. In its current state the test creates group using the > AzureHandler directly and indirectly via the Graph class and if both return > the same result, the test is successfull. This only works, if the Graph > class was able to initialize the access token for the AzureHandler class > correctly, which makes this test even more meaningful. OK > In comment 10 you wrote: > > > Please rename load_token_file to load_ids [...] > > done. OK > In comment 11 you wonderd: > > > Not sure about that list_teams() function. [...] > > Is that not just list groups? > > Yes it is and it is the way how it is documented on the Microsoft site. I > had the choice here to use the new beta API to get the list of teams as it > is described here: > https://docs.microsoft.com/en-us/graph/teams-list-all-teams#get-a-list-of- > groups-using-beta-apis > > The beta API would possibly perform better, but for now I decided against > it, because the beta API could be more unstable. I could have used the > list_groups functions from the base class at this point, but I would > consider that an unreasonable implementation decision as well, because it > would introduce a stronger binding between the two classes. It would also be > inconsistent with other calls within the Graph class in that it would then > use a different access token for the call. This makes the current > implementation more stable and future proof in the way, that we can still > modify code in the AzureHandler class as needed without having to worry > about compatibility to Graph. Yes, lets see what we need once we integrate this into the listener, ok for now So just these two things * fix login loop * remove page_size=500 ($top={page_size}) from list_teams, i think we don't need a paged request from our side, only server-side paging is required and that (works as far as i can tell)
Comment 11 was now resolved with version 2.0.2-106A~4.4.0.202103171457 Also all tests went successful, except for `92_office365.12_udm_user_license.master`, which keeps aborting with `There are no subscriptions available for testing in Azure Domain alias o365domain`. This seems unrelated to this change, was discussed in the chat and we will ignore that for now. Other tests only fail occasionally. I am still investigating, but I assume, that they are just flaky at the moment. If not I will reopen this ticket. Regarding comment 14 (and comment 11): The page_size is still in and I wonder why we should not keep it. I used it to try out how big a page can be and found that the maximum number of entries is 999. I could not find that number in the Microsoft documentation and I would argue, that it may change at some point in time without further notice. Obviously 999 would also be the most performant page_size as it would trigger fewer requests, but the function now defaults to a value of 500, so that there is a bit of a margin in case Microsoft changes the upper page limit just a bit. 500 is still performant and unless you are using the `terminaltest.py` application, you will never have to think about this limit anyways. On the other hand: If the day comes where you have to think about it, you are going to have with `terminaltest.py` an application to test possible values very quickly or you could even implement tests to verify the limit is still working. I will still close this issue for now, also because this function is not part of the acceptance criteria and does not have to be API stable or regularily tested in this early stage of development. You are welcome to reopen it, if you do not find these arguments convincing.
After a discussion we have decided to remove the page_size for now and the `$top` parameter in general for list_teams. Instead we now use Microsoft's default value. We can later tweak the performance by reintroducing it, but we will then also need tests. The latest version is now: 2.0.2-107A~4.4.0.202103171614
Introduced debugging code by accident, fixed in 2.0.2-108A~4.4.0.202103171709
I still have this login problem. -> /usr/share/ucs-test/92_office365/terminaltest.py --list_graph_users -d debug INFO:/usr/share/ucs-test/92_office365/terminaltest.py:proxy settings: {} INFO:/usr/share/ucs-test/92_office365/terminaltest.py:The access token for `o365domain` looks similar to: `eyJ0eXAiOi-trimmed-dLoY-p9WbA`. It is valid until 2021-03-17 18:11:15 INFO:/usr/share/ucs-test/92_office365/terminaltest.py:Next url: https://login.microsoftonline.com/3e7d9eb5-c3a1-4cfc-892e-a8ec29e45b77/oauth2/v2.0/token Starting new HTTPS connection (1): login.microsoftonline.com DEBUG:requests.packages.urllib3.connectionpool:Starting new HTTPS connection (1): login.microsoftonline.com https://login.microsoftonline.com:443 "POST /3e7d9eb5-c3a1-4cfc-892e-a8ec29e45b77/oauth2/v2.0/token HTTP/1.1" 401 714 DEBUG:requests.packages.urllib3.connectionpool:https://login.microsoftonline.com:443 "POST /3e7d9eb5-c3a1-4cfc-892e-a8ec29e45b77/oauth2/v2.0/token HTTP/1.1" 401 714 INFO:/usr/share/ucs-test/92_office365/terminaltest.py:The access token for `o365domain` looks similar to: `eyJ0eXAiOi-trimmed-dLoY-p9WbA`. It is valid until 2021-03-17 18:11:15 INFO:/usr/share/ucs-test/92_office365/terminaltest.py:Next url: https://login.microsoftonline.com/3e7d9eb5-c3a1-4cfc-892e-a8ec29e45b77/oauth2/v2.0/token Starting new HTTPS connection (1): login.microsoftonline.com DEBUG:requests.packages.urllib3.connectionpool:Starting new HTTPS connection (1): login.microsoftonline.com https://login.microsoftonline.com:443 "POST /3e7d9eb5-c3a1-4cfc-892e-a8ec29e45b77/oauth2/v2.0/token HTTP/1.1" 401 714 DEBUG:requests.packages.urllib3.connectionpool:https://login.microsoftonline.com:443 "POST /3e7d9eb5-c3a1-4cfc-892e-a8ec29e45b77/oauth2/v2.0/token HTTP/1.1" 401 714 INFO:/usr/share/ucs-test/92_office365/terminaltest.py:The access token for `o365domain` looks similar to: `eyJ0eXAiOi-trimmed-dLoY-p9WbA`. It is valid until 2021-03-17 18:11:15 INFO:/usr/share/ucs-test/92_office365/terminaltest.py:Next url: https://login.microsoftonline.com/3e7d9eb5-c3a1-4cfc-892e-a8ec29e45b77/oauth2/v2.0/token Starting new HTTPS connection (1): login.microsoftonline.com ... My feeling is that we should not use call_graph_api() in _login(). This kind of nested function (_login calls call_graph_api, call_graph_api call _login) is always error prone. So just make the request in _login directly
I do not think, that another call to requests.request would be less error prone and would like to stick to a all-or-nothing approach here. Also I believe, that this is only a dumb logic error, see: while url and retry: should probably better be: while url and (retry > 0): and it would also have failed the automatic retry on error 500 I am afraid.
(In reply to Max Pohle from comment #19) > I do not think, that another call to requests.request would be less error > prone and would like to stick to a all-or-nothing approach here. > > Also I believe, that this is only a dumb logic error, see: > while url and retry: > should probably better be: > while url and (retry > 0): not sure about that, but i think the problem is elsewhere surely we set retry = retry - 1 in _call_graph_api before calling _login, but there we call call_graph_api again and the default for retry is 1 again, so how is this gonna work?
The login issue is now fixed, the documentation has been revised and unified and the tests will now delete test objects after they finish. The version is: 2.0.2-109A~4.4.0.202103181633 Implementation note: The generic function `_call_graph_api` will now throw errors after too many unsuccessful retries and ends the loop by doing so. The decision was made that way, because either we have a generic function, which is resilient enough to withstand even edge-cases as logins or we should have the individual functions use `requests.request` directly, so that one broken function would not influence others. Both ways would be equally stable in my opinion, but we should not mix both concepts in order to keep the code consistent.
A minor fix was added. Fixed version is 2.0.2-110A~4.4.0.202103191118
> (1) > ... seems to be a timing issue, doing g = Graph(ucr, 'o365domain', 'o365domain') g.delete_team('b79f23f5-6835-4700-ab6b-21572548d888') a couple of seconds later works. > (2) just realized that get_all_aliases just lists the directories in /etc/univention-office365, i think we should stick with adconnection_aliases = AzureADConnectionHandler.get_adconnection_aliases() initialized_adconnections = [adconnection_alias for adconnection_alias in adconnection_aliases if AzureAuth.is_initialized(adconnection_alias)] (or maybe a new function for the initialized adconnections) please remove get_all_aliases
Hey Felix! (1) Yes, I have now added a timeout, similar to the other test. The version is now 2.0.2-113A~4.4.0.202103191627 (2) I had the function you are talking about in an earlier commit and I can revert that, so that it uses the univention config registry again and see, which aliases were 'initialized'. But I wonder for which reason I should, because to me it looks like unnecessary additional effort when setting up a new test environment. Could you explain further what the benefits would be? Also I cannot simply use the function you suggested for several reasons, one being, that we earlier found reasons for not using the `token.json` together with graph. But the `is_initialized` function you suggest uses the file to check for a field `consent_given`.
As discussed, please remove get_all_aliases().
Done in 2.0.2-114A~4.4.0.202103221217 and all tests passed.
c7b4535 Bug #52840: Added library functions for MS Teams 9cfd78e Bug #52840: Fixed get_http_proxies not found in listener 7621688 Bug #52840: Fixed generic problems in test setup 5641036 Bug #52840: Fixed first tests, lets retry on jenkins 7d413a2 Bug #52840: internal fix 1056624 Bug #52840: Renamed load_token_file to load_ids_file 46f3485 Bug #52840: Fixed bug in tests: graph_auth missing 962ede7 Bug #52840: Fixed 401_graph_backward_compatibility_test 75b55c2 Bug #52840: removed page_size via $top from list_teams cb0da79 Bug #52840: Removed accidently introduced debugging code 4474905 Bug #52840: Fixed login-loop, delete test users/groups/teams 4dd0649 Bug #52840: Fixed documentation and final refactoring efb470d Bug #52840: Fixed return value 'id' of create_group 1464a3a Bug #52840: Fixed delete group in test, added sleep 0d0bb2a Bug #52840: removed get_all_aliases from generic helper Graph handling is done in the new Graph class in modules/univention/office365/api/graph.py The listener implementation now uses an instance of Graph() instead of the previous AzureHandler(). The Graph class overloads methods from AzureHandler where applicable. univention-office365 2.0.2-155A~4.4.0.202109101246 Verified
Published in App 'office365' version 4.0