[Webkit-unassigned] [Bug 213177] [GTK][WPE] Add API to allow applications to handle the HTTP authentication credential storage

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=213177

Michael Catanzaro <mcatanzaro at gnome.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #402549|review?                     |review+
              Flags|                            |

--- Comment #10 from Michael Catanzaro <mcatanzaro at gnome.org> ---
Comment on attachment 402549
  --> https://bugs.webkit.org/attachment.cgi?id=402549
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=402549&action=review

Good API design. Good test (very comprehensive). Thanks for handling this.

> Source/WebKit/NetworkProcess/soup/NetworkProcessSoup.cpp:187
> +    if (auto* session = networkSession(sessionID))
> +        static_cast<NetworkSessionSoup&>(*session).setPersistentCredentialStorageEnabled(enabled);

Can we ASSERT() here that session is nonnull?

> Source/WebKit/UIProcess/API/glib/WebKitAuthenticationRequest.cpp:198
> + * if private browsing is enabled or persistent credential storage has been

if private browsing is enabled, or if persistent credential storage has been

> Source/WebKit/UIProcess/API/glib/WebKitAuthenticationRequest.cpp:229
> + * This should be used by applications handlig their own credentials

handling

> Source/WebKit/UIProcess/API/glib/WebKitAuthenticationRequest.cpp:277
> + * stored from a previous session. This should only be used by applications
> + * handling their own credential storage, when using the default WebKit credential
> + * storage webkit_authentication_request_get_proposed_credential() already contains
> + * previously stored credentials.

Sounds like: "this should only be used when handling own credential storage and when default credential storage contains previously stored credentials," which is not what you meant.

Fix: "This should only be used by applications handling their own credential storage. (When using the default WebKit credential storage, webkit_authentication_request_get_proposed_credential() already contains previously-stored credentials.)

> Source/WebKit/UIProcess/API/glib/WebKitAuthenticationRequest.cpp:278
> + * Passing a %NULL @credential will clear the proposed credential.

When might you want to clear the proposed credential?

> Tools/TestWebKitAPI/Tests/WebKitGLib/TestAuthentication.cpp:165
> -    g_assert_true(test->authenticationCancelledReceived);
> +    g_assert_true(test->m_authenticationCancelledReceived);

How about also checking: g_assert_false(test->m_authenticationSucceededReceived)?

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20200623/e667da96/attachment.htm>