Add warning message when email claim is missing from ID token

[Copilot]

This was an experiment by @EthanHeilman to test copilot capabilities

Plan: Better message in login to tell the user what claims they have (email)

• Explore repository structure and understand the issue
• Identify the IdentityString function that needs to be modified
• Build and test the current code
• Modify IdentityString function to provide clearer messaging about missing email claim
• Update test to reflect the new expected output when email is missing
• Add test case for when email is present (already covered)
• Run tests to verify changes work correctly
• Add opkssh binary to .gitignore
• Manually verify the output changes
• Request code review
• Address code review feedback
• Update warning message with more detailed guidance
• Refactor Mocks function to accept optional ExtraClaims
• Fix linter issues (gofmt)
• Apply fmt.Sprintf pattern consistently

Changes Made

Modified IdentityString function in commands/login.go:

• Updated warning message to provide more helpful information:
  • Mentions that policies based on email will not work
  • Suggests checking client config (~/.opk/config.yml) for correct scopes
  • Directs users to the specific OpenID Provider configuration
• Applied fmt.Sprintf pattern with multi-line strings to both if and else blocks for consistency

Refactored Mocks function in commands/login_test.go:

• Now accepts optional extraClaims parameter (variadic)
• Defaults to including email claim for backward compatibility
• Allows tests to specify custom ExtraClaims including empty maps

Updated TestIdentityString in commands/login_test.go:

• Simplified test for "without email claim" to use the refactored Mocks function
• Removed duplicated mock setup code
• Updated assertions to check for the new warning message text
• Fixed gofmt formatting issues (trailing whitespace and comment alignment)

Output Examples:

With email claim:

Email, sub, issuer, audience: 
arthur.aardvark@example.com me https://accounts.example.com test_client_id

Without email claim (improved warning message):

WARNING: Email claim is missing from ID token. Policies based on email will not work.
Check if your client config (~/.opk/config.yml) has the correct scopes configured for this OpenID Provider.
Sub, issuer, audience:
me https://accounts.example.com test_client_id

All tests pass successfully and linter passes with 0 issues.

Original prompt

---

This section details on the original issue you should resolve

<issue_title>Better message in login to tell the user what claims they have (email)</issue_title>
<issue_description>"could a check be added to ensure that the email claim is present in the ID token? Currently, users are just not able to login and this log message:

Keys generated for identity
Sub, issuer, audience:
is not really indicative that something is wrong."

Requested in #38 (comment)</issue_description>

<agent_instructions>Do your best job</agent_instructions>

Comments on the Issue (you are @copilot in this section)

Fixes #103

---

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

[EthanHeilman]

@copilot See my review and the breaking tests

[EthanHeilman]

@copilot The code you wrote seems to have broken the linter. Fix the linter issue

[Copilot]

@copilot The code you wrote seems to have broken the linter. Fix the linter issue

Fixed the linter issue in commit 4b8c4e1. The problem was trailing whitespace and misaligned comments in login_test.go. Ran gofmt to fix formatting - linter now passes with 0 issues.

[Copilot]

Pull Request Overview

This PR improves the user experience when the email claim is missing from the ID token during login by adding a clear warning message that explains the implications and provides actionable troubleshooting guidance.

Key changes:

• Enhanced the warning message in IdentityString to explain that policies based on email won't work and direct users to check their client configuration
• Refactored the Mocks test helper to accept optional extra claims for more flexible testing
• Added comprehensive test coverage for both scenarios (with and without email claim)

Reviewed Changes

Copilot reviewed 2 out of 3 changed files in this pull request and generated 2 comments.

File	Description
commands/login.go	Updated warning message when email claim is missing to provide actionable guidance
commands/login_test.go	Refactored Mocks function to support custom claims and added test case for missing email scenario

---

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

[EthanHeilman]

LGTM

[EthanHeilman]

This was an experiment to see how copilot would handle a very simple issue. On one hand, it did end up solving the issue and didn't require much supervision. On the other hand, I probably could have done it for the same time I spent going back and forth with copilot.

Overall, the benefit seems to be:

• I don't need a 30 minute block of attention to get it done. Instead I can just, check in for 5 minutes between meetings and point it in the right direction.

• I can review code I didn't write which generally results in better reviews because I don't bring the assumptions I used to write the code to the review.

• I can work on a different branch on my local computer and not have to switch between the larger branch I am working on and this tiny feature branch. This is the biggest advantage in my mind.