Skip to content

Conversation

Copilot
Copy link
Contributor

@Copilot Copilot AI commented Oct 20, 2025

Documentation Fix: Update CookieAuthenticationOptions XML Comments

Plan

  • Understand the issue: XML doc comment on ExpireTimeSpan property incorrectly suggests using CookieOptions.Expires as a separate control, but this property is actually ignored/validated against
  • Explore repository structure and understand the code
  • Run initial build and tests - all pass ✓
  • Update XML documentation for ExpireTimeSpan property
  • Build and verify changes - all tests pass ✓
  • Run CodeQL security check - no issues ✓
  • Address feedback: Use CookieBuilder.Expiration instead of CookieOptions.Expires for consistency ✓

Changes Summary

Updated the XML documentation comment for the ExpireTimeSpan property in CookieAuthenticationOptions.cs to correctly reflect the implementation.

Final change:

  • Changed reference from CookieOptions.Expires to CookieBuilder.Expiration to be consistent with:
    • The error message: "Cookie.Expiration is ignored, use ExpireTimeSpan instead."
    • Line 48 of the same file which references CookieBuilder.Expiration

Documentation now states:

/// This property should be used instead of <see cref="CookieBuilder.Expiration"/>, which is ignored.

This ensures consistency throughout the codebase and aligns with how developers actually interact with the API (via Cookie.Expiration where Cookie is of type CookieBuilder).

Original prompt

This section details on the original issue you should resolve

<issue_title>Update CookieAuthenticationOptions comments</issue_title>
<issue_description>### Is there an existing issue for this?

  • I have searched the existing issues

Describe the bug

Correct the currently incorrect comments in the source code for the Cookie Handler.

In this file:
src/Security/Authentication/Cookies/src/CookieAuthenticationOptions.cs

It says:

    /// <summary>
    /// <para>
    /// Controls how much time the authentication ticket stored in the cookie will remain valid from the point it is created.
    /// The expiration information is stored in the protected cookie ticket. Because of that an expired cookie will be ignored
    /// even if it is passed to the server after the browser should have purged it.
    /// </para>
    /// <para>
    /// This is separate from the value of <see cref="CookieOptions.Expires"/>, which specifies
    /// how long the browser will keep the cookie.
    /// </para>
    /// </summary>
    public TimeSpan ExpireTimeSpan { get; set; }

It mentions that I can use the CookieOptions.Expires, however, when we try it, it throws an exception:

OptionsValidationException: Cookie.Expiration is ignored, use ExpireTimeSpan instead.`
builder.Services.AddOptions<CookieAuthenticationOptions>(authenticationScheme).Validate(o => o.Cookie.Expiration == null, "Cookie.Expiration is ignored, use ExpireTimeSpan instead.");

Expected Behavior

This is fine, but i wish the comments in the CookieAuthenticationOptions was updated to better reflect the reality and if you want to control the cookie lifetime separately, you have the MaxAge option to use instead?
or are there a better option if I want to set the cookie expire time separately?

Steps To Reproduce

No response

Exceptions (if any)

No response

.NET Version

No response

Anything else?

No response</issue_description>

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

@MackinnonBuck We should clarify the XML docs to remove this remark and instead state that this property should be used instead of `CookieOptions.Expires`.

In CookieAuthenticationHandler, we override CookieOptions.Expires anyway based on the value specified in CookieAuthenticationOptions.ExpireTimeSpan.</comment_new>

Fixes #64074


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Co-authored-by: MackinnonBuck <10456961+MackinnonBuck@users.noreply.github.com>
@Copilot Copilot AI changed the title [WIP] Update comments for CookieAuthenticationOptions Update CookieAuthenticationOptions XML documentation to clarify ExpireTimeSpan usage Oct 20, 2025
Copilot finished work on behalf of MackinnonBuck October 20, 2025 17:47
@Copilot Copilot AI requested a review from MackinnonBuck October 20, 2025 17:47
@MackinnonBuck MackinnonBuck marked this pull request as ready for review October 20, 2025 19:05
@Copilot Copilot AI review requested due to automatic review settings October 20, 2025 19:05
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR corrects misleading XML documentation for the ExpireTimeSpan property in CookieAuthenticationOptions. The original documentation incorrectly implied that CookieOptions.Expires could be used separately to control browser cookie lifetime, when in fact attempting to set it throws a validation exception.

Key Changes:

  • Updated XML documentation to clarify that ExpireTimeSpan should be used instead of CookieOptions.Expires, not in addition to it
  • Removed misleading language that suggested CookieOptions.Expires could be used to separately control browser cookie lifetime
  • Aligned documentation with actual implementation behavior where the handler overrides cookie expiration based on ExpireTimeSpan

…documentation

Co-authored-by: MackinnonBuck <10456961+MackinnonBuck@users.noreply.github.com>
Copilot finished work on behalf of MackinnonBuck October 22, 2025 23:16
@Copilot Copilot AI requested a review from MackinnonBuck October 22, 2025 23:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Update CookieAuthenticationOptions comments

3 participants