Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Backend: Prefill local account oobe macos setup #17401

Closed
3 tasks done
georgekarrv opened this issue Mar 6, 2024 · 8 comments
Closed
3 tasks done

Backend: Prefill local account oobe macos setup #17401

georgekarrv opened this issue Mar 6, 2024 · 8 comments
Assignees
Labels
~backend Backend-related issue. #g-mdm MDM product group P2 Prioritize as urgent :release Ready to write code. Scheduled in a release. See "Making changes" in handbook. ~sub-task A technical sub-task that is part of a story. (Not QA'd. Not estimated.)
Milestone

Comments

@georgekarrv
Copy link
Member

georgekarrv commented Mar 6, 2024

  • Make sure this is addressed: Add enable_release_device_manually setting to team and no-team #17698 (comment)

  • Make sure this is addressed: "Set await_device_configured to true in the DEP profile for existing users (migration)"

    • I think this can simply enqueue a MacosSetupAssistantUpdateAllProfiles worker job which will re-register all existing DEP profiles with await_device_configured to true
    • Does that mean that existing DEP-enrolled devices will now need to be explicitly released? The docs mention it blocks in Setup Assistant so presumably devices that are already past this assistant will be ok, but worth double-checking in tests. EDIT: Roberto confirmed that this is ok, it only affect the setup assistant: Send DeviceConfigured MDM command after DEP enrollment #17737 (comment)
  • From the Slack convo with Noah, we should test this scenario

    me: the device may go offline immediately after we sent the “install fleetd/bootstrap/etc.” commands
    Noah: Do we know if macOS shows some message to the user if this happens? We should test it.

    The device is blocked on a screen until released.

@georgekarrv georgekarrv added :product Product Design department (shows up on 🦢 Drafting board) #g-mdm MDM product group ~sub-task A technical sub-task that is part of a story. (Not QA'd. Not estimated.) ~backend Backend-related issue. labels Mar 6, 2024
@georgekarrv
Copy link
Member Author

@georgekarrv georgekarrv added :release Ready to write code. Scheduled in a release. See "Making changes" in handbook. and removed :product Product Design department (shows up on 🦢 Drafting board) labels Mar 11, 2024
@lukeheath lukeheath added this to the 4.48.0-tentative milestone Mar 11, 2024
@georgekarrv georgekarrv added the P2 Prioritize as urgent label Mar 14, 2024
@roperzh
Copy link
Member

roperzh commented Mar 14, 2024

@mna @ghernandez345 one thing I want to note that might help, the nano queue has a priority field:

`priority` tinyint(4) NOT NULL DEFAULT '0',

that might come in handy to achieve the order specified in the issue:

image

@mna
Copy link
Member

mna commented Mar 19, 2024

@roperzh Just to sanity-check my understanding with you for that priority approach, as I don't think I grasp the nanomdm's flow as well as you:

  • This is where the next command to send to the device is retrieved:
    func (m *MySQLStorage) RetrieveNextCommand(r *mdm.Request, skipNotNow bool) (*mdm.Command, error) {
    command := new(mdm.Command)
    err := m.db.QueryRowContext(
    r.Context, `
    SELECT c.command_uuid, c.request_type, c.command
    FROM nano_enrollment_queue AS q
    INNER JOIN nano_commands AS c
    ON q.command_uuid = c.command_uuid
    LEFT JOIN nano_command_results r
    ON r.command_uuid = q.command_uuid AND r.id = q.id
    WHERE q.id = ?
    AND q.active = 1
    AND (r.status IS NULL OR (r.status = 'NotNow' AND NOT ?))
    ORDER BY
    q.priority DESC,
    q.created_at
    LIMIT 1;`,
    r.ID, skipNotNow,
    ).Scan(&command.CommandUUID, &command.Command.RequestType, &command.Raw)
    if errors.Is(err, sql.ErrNoRows) {
    return nil, nil
    }
    if err != nil {
    return nil, err
    }
    return command, nil
    }
  • It does order by priority DESC, but it skips any non-NULL result status (and possibly NotNow ones too)
  • It does not guarantee that the previous command is completed successfully before moving on to the next in priority, it only guarantees delivery ordering
  • However, the only non-NULL statuses possible are: "Acknowledged", "Error", "CommandFormatError", "Idle" and "NotNow", and nanomdm does not store "Idle" statuses, so that only leaves success ("Acknowledged"), failure (both errors) and "NotNow"
  • Do we retry commands that result in errors? If not, then this is a final state and I think the only problematic status (that could mess the ordering) would be "NotNow"?

If that's the case (that only NotNow is a problem), I think we could relatively easily handle this case by not returning the next command when we receive a NotNow and there is at least one command available with a priority > 0. Unless of course we want to release the device only after osquery verification, but I don't think that's the case?

@roperzh
Copy link
Member

roperzh commented Mar 19, 2024

@mna what you described is exactly my understanding!

nanomdm does not store "Idle" statuses

to add more color to this, in case it helps. Idle is how the device starts the MDM session, so it's never a "result" of another command.

Do we retry commands that result in errors? If not, then this is a final state and I think the only problematic status (that could mess the ordering) would be "NotNow"?

currently we don't! we do retry things (eg: failed profiles) but we use a new command to retry those, so I think what you described is exactly right.

If that's the case (that only NotNow is a problem), I think we could relatively easily handle this case by not returning the next command when we receive a NotNow and there is at least one command available with a priority > 0. Unless of course we want to release the device only after osquery verification, but I don't think that's the case?

this sounds reasonable to me. I'm not sure if this is a problem, but the only danger I see with using priority is that every time we enqueue a command, we also send a push notification to the device.

if we don't enqueue all the commands we need delivered in order in the same transaction, then the device could reach out and get commands out of order (if that makes sense)

@mna
Copy link
Member

mna commented Mar 19, 2024

@roperzh

the only danger I see with using priority is that every time we enqueue a command, we also send a push notification to the device. if we don't enqueue all the commands we need delivered in order in the same transaction, then the device could reach out and get commands out of order (if that makes sense)

Ah right, good call... That's going to be a problem.

@roperzh
Copy link
Member

roperzh commented Mar 19, 2024

@mna that one is tricky... spitballing anything that comes to mind, in case any of this helps:

  • In addition to priority, there's also the active, I wonder if we could enqueue all the commands as active = 0 and switch them to 1 when we know we're ready? (seems a bit sketchy?)
  • we have the option to not send a push notification when we enqueue a command, but this one is a bit risky, we can't be 100% sure that the device won't reach out on it's own.
  • If things get too complicated, maybe we could talk with Marko to reduce the scope? I think to accomplish the goal of this task we don't really need an exact order, we need:
    • for all profiles to be installed before the device is released (so any account policies like password length are set when the user creates the account)
    • for AccountConfiguration to be sent (so the username and fullname are pre-filled)

@mna
Copy link
Member

mna commented Mar 19, 2024

@roperzh thanks for helping thinking this through, I know you're super busy with high-priority work too, I appreciate it! Did a bit more digging and it looks like all DEP commands get sent here:

func (a *AppleMDM) runPostDEPEnrollment(ctx context.Context, args appleMDMArgs) error {

So I think it would be doable to send them all in one transaction with the relevant priorities in this scenario. Only thing it wouldn't cover, and I'm not sure how easily we could, is the delivery of profiles - I assume those are profiles for the team the host enrolls in? And those run in the ReconcileAppleProfiles job so it's async and runs every 30s. (maybe by enqueuing another worker job to run later, and hope that by then - and with the help of some retries - all profiles have been delivered)

I'll try to go down that route, let me know if you see any concerns/blockers with it but otherwise I'll try to leave you alone! :D

@fleet-release
Copy link
Contributor

In the cloud city,
Fleet eases Mac setup steps,
Devices freed, users rest.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
~backend Backend-related issue. #g-mdm MDM product group P2 Prioritize as urgent :release Ready to write code. Scheduled in a release. See "Making changes" in handbook. ~sub-task A technical sub-task that is part of a story. (Not QA'd. Not estimated.)
Development

No branches or pull requests

5 participants