Closed lukeheath closed 1 year ago
From estimation:
403, T_C_NOT_SIGNED
is returned on any request. POST https://mdmenrollment.apple.com/server/devices
, likely hourly, so we can show hosts that have not come online yet. We need to confirm this endpoint will return the error we need. @mna couple of things in case they are helpful:
HTTP 403 Forbidden
+ T_C_NOT_SIGNED
in the body in all DEP endpointsgithub.com/micromdm/nanodep/godep
to consume the DEP APIs, this is used in a cron and in an endpoint (I'm not sure if we want to keep the endpoint around). Should we consider adding a wrapper around it? Whatever you decide, this is how the auth error is handled in the library, I think it will come in handy: https://github.com/micromdm/nanodep/blob/0cbf4ee653854a3c82a9e42841e8bcf937b744ba/godep/client.go#L123-L127@noahtalerman One thing I want to point out to make sure we're ok with this behaviour : let's say we detect a change of Apple's terms, so we set the flag and print the banner in the UI telling the user that they must go to Apple's BM website to accept them. We have no way to know if they actually did accept them until the next time we make an actual API request to Apple BM and clear the flag as a result of its success. This means that after accepting the terms on Apple's website, the user will still see the banner in Fleet.
I wonder if the message shown in the banner should account for this scenario and mention that it will automatically detect that the terms have been accepted the next time it makes a request? Or even better, tell the user to visit the list of hosts pending enrollment to force an API call, so that the banner can be removed?
Also, note that I added this to the spec:
Ensure value cannot be set manually via PATCH /config. It makes no sense to make it user-modifiable.
I.e. the new flag we set for the change of Apple's terms cannot be set manually by the user via the standard Fleet API call.
We have no way to know if they actually did accept them until the next time we make an actual API request to Apple BM and clear the flag as a result of its success. This means that after accepting the terms on Apple's website, the user will still see the banner in Fleet.
I wonder if the message shown in the banner should account for this scenario
Hmm, great point. @chris-mcgillicuddy do you think you can help us tweak the message in the banner? Martin has proposed good ideas in his above comment.
Here's the Figma page with the banner: https://www.figma.com/file/hdALBDsrti77QuDNSzLdkx/%F0%9F%9A%A7-Fleet-EE-(dev-ready%2C-scratchpad)?node-id=10694%3A317922&t=rzA0hmAJlV4PIGdu-1
@mna how do you think we should tweak the message?
@noahtalerman I was thinking something like that added to the current message:
Once that is done, visit
<link to mdm pending enrollment hosts>
to automatically remove this notification.
Looks good to me! I tweaked your message a bit to make it shorter: Done? Go to [Hosts](link to hosts) to remove this banner.
@mna what do you think? Is it still clear?
@noahtalerman Yeah LGTM! Just to be clear, that's not the generic list of hosts page, it's the one that shows hosts pending MDM enrollment, right ? (because that's the page that will make a request to the Apple BM API, confirming that the terms have been accepted as the call won't fail anymore)
Just to be clear, that's not the generic list of hosts page, it's the one that shows hosts pending MDM enrollment, right ?
@mna I did mean the Hosts page (generic list).
My understanding is that the Hosts page is a page that will make a request to the Apple BM API. This is because the Hosts page displays hosts pending enrollment. I could be wrong here...
My understanding is that the Hosts page is a page that will make a request to the Apple BM API. This is because the Hosts page displays hosts pending enrollment.
@noahtalerman gotcha, yeah that makes sense, I wasn't aware the the pending enrollment stuff was part of the main Hosts page. Yeah as long as it directs to somewhere that will make that Apple BM API request, it's all good!
@lukeheath will the Hosts page make a request to Apple Business Manager (ABM) API?
Martin uncovered this problem: It's possible that the ABM warning banner won't be cleared after a user has agreed to ABMs new terms.
We think linking the user the Hosts page solves the problem because this page will make a request to an ABM API.
@noahtalerman, if the Hosts page is the correct destination, then the updated banner copy looks good to me!
@noahtalerman @mna
I do not know precisely when we will hit the APIs until we're further along. I do not think we will need to call it on the hosts page, because we will be ingesting that data separately. Perhaps an hourly CRON job that ingests "Pending" hosts? @gillespi314 is working on that in #8876
@mna Will any request to the Apple device management API work, or does it have to be something Business Manager-specific?
Will any request to the Apple device management API work, or does it have to be something Business Manager-specific?
@lukeheath It would be any endpoint that uses the Apple BM server token to authenticate, AIUI (although it's not 100% clear in the Apple docs as they only mention "The organization has not accepted the latest terms and conditions of the program.", which I think refers to the Device Enrollment Program). Also, we mention in the banner that an Apple BM administrator can do that and we link the user to business.apple.com, it's probably fair to assume it needs to be BM-related.
I do not think we will need to call it on the hosts page
@lukeheath got it. If this is the case, we'll need to rethink the copy in the banner.
I think removing the Done? Go to [Hosts](link to hosts) to remove this banner.
from the copy is acceptable UX for now.
Later, we can address this problem: It's possible that the ABM warning banner won't be cleared after a user has agreed to ABMs new terms.
@noahtalerman Sounds good. Once we have a better understanding of how and when we are interacting with the ABM API we can craft a better solution.
@roperzh correct me if I'm wrong, but it looks like the fork of nanodep that we use is quite different than the link to error handling that you provided above (https://github.com/micromdm/nanodep/blob/0cbf4ee653854a3c82a9e42841e8bcf937b744ba/godep/client.go#L123-L127) ?
I.e. where micromdm/nanodep uses a custom HTTP client type, our fork uses a standard stdlib HTTP client where everything happens in the Transport's RoundTrip
: https://github.com/fleetdm/nanodep/blob/main/client/transport.go#L190-L207 (and there's no custom HTTPError type).
@roperzh oh god, concerning the comment above, I think it's super confusing because there is nanodep/godep/NewClient
and nanodep/client/NewClient
, and the one you mentioned was the godep one, while the one I saw in the code was the client
one :exploding_head: . I assume the correct one is the godep
one with the error handling that you mentioned, so I'll make sure all http clients based on depStorage
use this one, and will try to hook into errors returned by this one...
@noahtalerman
Regarding the warning banner in fleetctl
:
Return the following warning message in all fleetctl commands:
Your organization can’t automatically enroll macOS hosts until you accept the new terms and conditions for Apple Business Manager (ABM). An ABM administrator can accept these terms. Go to ABM: https://business.apple.com/
What I've done is that this check is done whenever a fleetctl
command is about to make an authenticated Fleet API call. This is so that we don't attempt to make this check when e.g. running fleetctl help
or other commands that don't even communicate with the Fleet API, as those commands can work without a fleet instance running. From what I've seen, this is consistent with the other fleetctl
checks we make (e.g. version mismatch between fleetctl and fleet, and fleet license expired).
The output looks like this (with the warning being printed in red like the license expired banner):
$ ./build/fleetctl get labels
Your organization can’t automatically enroll macOS hosts until you accept the new terms and conditions for Apple Business Manager (ABM). An ABM administrator can accept these terms. Go to ABM: https://business.apple.com/
+---------------+----------+--------------------------------+--------------------------------+
| NAME | PLATFORM | DESCRIPTION | QUERY |
+---------------+----------+--------------------------------+--------------------------------+
| All Hosts | | All hosts which have enrolled | select 1; |
| | | in Fleet | |
+---------------+----------+--------------------------------+--------------------------------+
this is consistent with the other fleetctl checks we make
@mna looks good! Being consistent here makes sense to me 👍
@mna wow, yes you're right it's like one of the clients implements only a subset of the DEP apis, and you can use the other to make the other calls :/
@roperzh yeah I was confused at first, but turns out that's fine, I added the API calls we need to the godep
client so we can have the same error-handling logic for all our use-cases. The nanodep
PR is here: https://github.com/fleetdm/nanodep/pull/2
/sync_a_list_of_devices) API.
Haiku: New terms accepted Safeguarding devices secure Peace of mind for users
Problem
Users need to know when to accept new terms and conditions for Apple Business Manager. Users won't be able to automatically enroll new macOS hosts until they accept the terms.
Related
Requirements
See requirements in epic.
Reference
Copied from this comment by Roberto.
And, under "Deployment Scenarios"
It's unclear to me from the docs if you get a
403
on any endpoint or only in authorization-related endpoints.Also documented directly in Apple Business Manager API docs.
Notes
Tasks
1
mdm.apple_bm_terms_expired: boolean
, so that the default value isfalse
and needs no action, and is set totrue
when an action is required and the banner must be printed.GET /config
.PATCH /config
. It makes no sense to make it user-modifiable.2
403, T_C_NOT_SIGNED
is returned.3
fleetctl
to check for this value.4
PATCH /config
, no need to document it as an input of this API.