Merge pull request #282 from OneNoteDev/feature/unwriteable-cookie-detect

Feature/unwriteable cookie detect

Author: leakymemory

src/scripts/clipperUI/panels/signInPanel.tsx

@@ -12,7 +12,7 @@ import {ComponentBase} from "../componentBase";
 import {Status} from "../status";
 
 interface SignInPanelState {
-	errorDescriptionShowing?: boolean;
+	debugInformationShowing?: boolean;
 }
 
 interface SignInPanelProps extends ClipperStateProp {
@@ -21,7 +21,7 @@ interface SignInPanelProps extends ClipperStateProp {
 
 class SignInPanelClass extends ComponentBase<SignInPanelState, SignInPanelProps> {
 	getInitialState() {
-		return { errorDescriptionShowing: false };
+		return { debugInformationShowing: false };
 	}
 
 	onSignInMsa() {
@@ -51,61 +51,96 @@ class SignInPanelClass extends ComponentBase<SignInPanelState, SignInPanelProps>
 		}
 	}
 
-	getSignInFailureDetected(): boolean {
+	signInAttempted(): boolean {
 		return !!this.props.clipperState.userResult && !!this.props.clipperState.userResult.data
-			&& this.props.clipperState.userResult.data.updateReason === UpdateReason.SignInAttempt
+			&& this.props.clipperState.userResult.data.updateReason === UpdateReason.SignInAttempt;
+	}
+
+	signInFailureContainsErrorDescription(): boolean {
+		return this.signInAttempted()
+			&& this.props.clipperState.userResult.data.errorDescription
 			// Right now we are only showing the error panel for OrgId errors since they tend to
 			// be a little more actionable to the user, or at least a little more helpful.
-			&& this.props.clipperState.userResult.data.errorDescription
 			&& this.props.clipperState.userResult.data.errorDescription.indexOf("OrgId") === 0;
 	}
 
-	errorDescriptionControlHandler() {
-		this.setState({ errorDescriptionShowing: !this.state.errorDescriptionShowing });
+	signInFailureThirdPartyCookiesBlocked(): boolean {
+		return this.signInAttempted()
+			&& !this.props.clipperState.userResult.data.writeableCookies;
+	}
+
+	debugInformationControlHandler() {
+		this.setState({ debugInformationShowing: !this.state.debugInformationShowing });
 	}
 
-	errorInformationToggle() {
-		if (this.getSignInFailureDetected()) {
-			return <div className="signInErrorToggleInformation">
-				<a id={Constants.Ids.signInErrorMoreInformation} {...this.enableInvoke(this.errorDescriptionControlHandler, 10) }>
+	signInErrorDetected() {
+		return this.signInFailureContainsErrorDescription() || this.signInFailureThirdPartyCookiesBlocked();
+	}
+
+	errorMoreInformationTogggle() {
+		if (this.signInErrorDetected()) {
+			return <div id="signInErrorToggleInformation">
+				<a id={Constants.Ids.signInErrorMoreInformation} {...this.enableInvoke(this.debugInformationControlHandler, 10) }>
 					<img id={Constants.Ids.signInToggleErrorDropdownArrow} src={ExtensionUtils.getImageResourceUrl("dropdown_arrow.png")} />
 					<span id={Constants.Ids.signInToggleErrorInformationText}
 						style={Localization.getFontFamilyAsStyle(Localization.FontFamily.Light)}>
-						{this.state.errorDescriptionShowing
+						{this.state.debugInformationShowing
 							? Localization.getLocalizedString("WebClipper.Label.SignInUnsuccessfulLessInformation")
 							: Localization.getLocalizedString("WebClipper.Label.SignInUnsuccessfulMoreInformation")
 						}
 					</span>
 				</a>
+				{this.debugInformation()}
 			</div>;
 		}
 
 		return undefined;
 	}
 
-	errorInformationDescription() {
-		if (this.getSignInFailureDetected() && this.state.errorDescriptionShowing) {
-			return <div id={Constants.Ids.signInErrorDescription}>
-				<span className={Constants.Ids.signInErrorDescriptionContainer} style={Localization.getFontFamilyAsStyle(Localization.FontFamily.Light)}>
+	debugInformation() {
+		if (this.signInErrorDetected() && this.state.debugInformationShowing) {
+			return <div id={Constants.Ids.signInErrorDebugInformation}>
+				<span id={Constants.Ids.signInErrorDebugInformationDescription} style={Localization.getFontFamilyAsStyle(Localization.FontFamily.Light)}>
 					{this.props.clipperState.userResult.data.errorDescription}
 				</span>
-				<div className={Constants.Ids.signInErrorDebugInformationContainer}
-					style={Localization.getFontFamilyAsStyle(Localization.FontFamily.Light)}>
-					<ul className={Constants.Ids.signInErrorDebugInformationList}>
+				<div id={Constants.Ids.signInErrorDebugInformationContainer} style={Localization.getFontFamilyAsStyle(Localization.FontFamily.Light)}>
+					<ul id={Constants.Ids.signInErrorDebugInformationList}>
 						<li>{ClientType[this.props.clipperState.clientInfo.clipperType]}: {this.props.clipperState.clientInfo.clipperVersion}</li>
 						<li>ID: {this.props.clipperState.clientInfo.clipperId}</li>
 						<li>USID: {Clipper.getUserSessionId()}</li>
 					</ul>
 				</div>
 			</div>;
 		}
+	}
+
+	getErrorDescription() {
+		if (this.signInFailureContainsErrorDescription()) {
+			return this.props.clipperState.userResult.data.errorDescription;
+		} else if (this.signInFailureThirdPartyCookiesBlocked()) {
+			return <div>
+				<div class={Constants.Ids.signInErrorCookieInformation}>{Localization.getLocalizedString("WebClipper.Error.CookiesDisabled.Line1")}</div>
+				<div class={Constants.Ids.signInErrorCookieInformation}>{Localization.getLocalizedString("WebClipper.Error.CookiesDisabled.Line2")}</div>
+			</div>;
+		}
 
 		return undefined;
 	}
 
-	render() {
-		let signInDescription = this.getSignInDescription();
+	errorInformationDescription() {
+		if (this.signInErrorDetected()) {
+			return <div id={Constants.Ids.signInErrorDescription}>
+				<span id={Constants.Ids.signInErrorDescriptionContainer} style={Localization.getFontFamilyAsStyle(Localization.FontFamily.Light)}>
+					{this.getErrorDescription()}
+				</span>
+				{this.errorMoreInformationTogggle()}
+			</div>;
+		}
+
+		return undefined;
+	}
 
+	render() {
 		return (
 			<div id={Constants.Ids.signInContainer}>
 				<div className="signInPadding">
@@ -119,10 +154,9 @@ class SignInPanelClass extends ComponentBase<SignInPanelState, SignInPanelProps>
 					<div className="signInDescription">
 						<span id={Constants.Ids.signInText}
 							style={Localization.getFontFamilyAsStyle(Localization.FontFamily.Light)}>
-							{signInDescription}
+							{this.getSignInDescription()}
 						</span>
 					</div>
-					{this.errorInformationToggle()}
 					{this.errorInformationDescription()}
 					<a id={Constants.Ids.signInButtonMsa} {...this.enableInvoke(this.onSignInMsa, 11, AuthType.Msa)}>
 						<div className="wideButtonContainer">

src/scripts/constants.ts

@@ -222,6 +222,9 @@ export module Constants {
 		export var signInButtonMsa = "signInButtonMsa";
 		export var signInButtonOrgId = "signInButtonOrgId";
 		export var signInContainer = "signInContainer";
+		export var signInErrorCookieInformation = "signInErrorCookieInformation";
+		export var signInErrorDebugInformation = "signInErrorDebugInformation";
+		export var signInErrorDebugInformationDescription = "signInErrorDebugInformationDescription";
 		export var signInErrorDebugInformationContainer = "signInErrorDebugInformationContainer";
 		export var signInErrorDebugInformationList = "signInErrorDebugInformationList";
 		export var signInErrorDescription = "signInErrorDescription";
@@ -345,7 +348,6 @@ export module Constants {
 	}
 
 	export module Urls {
-		// export var serviceDomain = "https://minint-per7o4f.redmond.corp.microsoft.com";
 		export var serviceDomain = "https://www.onenote.com";
 
 		export var augmentationApiUrl = serviceDomain + "/onaugmentation/clipperextract/v1.0/";

src/scripts/cookieUtils.ts

@@ -1,7 +1,7 @@
 import {ObjectUtils} from "./objectUtils";
 
-export module CookieUtils {
-	export function readCookie(cookieName: string, doc?: Document) {
+export class CookieUtils {
+	public static readCookie(cookieName: string, doc?: Document) {
 		if (ObjectUtils.isNullOrUndefined(doc)) {
 			doc = document;
 		}

src/scripts/extensions/authenticationHelper.ts

@@ -53,36 +53,35 @@ export class AuthenticationHelper {
 			}
 
 			let getUserInformationFunction = () => {
-				return new Promise<ResponsePackage<string>>((resolve2, reject2) => {
-					this.getClipperInfoCookie(clipperId).then((cookie) => {
-						this.retrieveUserInformation(clipperId, cookie).then((result) => {
-							resolve2(result);
-						}, (errorObject) => {
-							reject2(errorObject);
-						});
-					});
+				return this.getClipperInfoCookie(clipperId).then((cookie) => {
+					return this.retrieveUserInformation(clipperId, cookie);
 				});
 			};
 
 			let getInfoEvent: Log.Event.PromiseEvent = new Log.Event.PromiseEvent(Log.Event.Label.GetExistingUserInformation);
 			getInfoEvent.setCustomProperty(Log.PropertyName.Custom.UserInformationStored, !!storedUserInformation);
 			this.clipperData.getFreshValue(ClipperStorageKeys.userInformation, getUserInformationFunction, updateInterval).then((response: TimeStampedData) => {
-				getInfoEvent.setCustomProperty(Log.PropertyName.Custom.FreshUserInfoAvailable, !!response);
+				let isValidUser = this.isValidUserInformation(response.data);
+				getInfoEvent.setCustomProperty(Log.PropertyName.Custom.FreshUserInfoAvailable, isValidUser);
 
-				if (response) {
-					this.user.set({ user: response.data, lastUpdated: response.lastUpdated, updateReason: updateReason });
+				let writeableCookies = this.isThirdPartyCookiesEnabled(response.data);
+				getInfoEvent.setCustomProperty(Log.PropertyName.Custom.WriteableCookies, writeableCookies);
+
+				if (isValidUser) {
+					this.user.set({ user: response.data, lastUpdated: response.lastUpdated, updateReason: updateReason, writeableCookies: writeableCookies });
 				} else {
-					this.user.set({ updateReason: updateReason });
+					this.user.set({ updateReason: updateReason, writeableCookies: writeableCookies });
 				}
 
-				resolve(this.user.get());
 			}, (error: OneNoteApi.GenericError) => {
 				getInfoEvent.setStatus(Log.Status.Failed);
 				getInfoEvent.setFailureInfo(error);
+
 				this.user.set({ updateReason: updateReason });
-				resolve(this.user.get());
 			}).then(() => {
 				this.logger.logEvent(getInfoEvent);
+
+				resolve(this.user.get());
 			});
 		});
 	}
@@ -157,8 +156,8 @@ export class AuthenticationHelper {
 
 			HttpWithRetries.post(userInfoUrl, postData, headers).then((request: XMLHttpRequest) => {
 				let response = request.response;
-				// The false case is expected behavior if the user has not signed in or credentials have expired
-				resolve({ parsedResponse: this.isValidUserInformationJsonString(response) ? response : undefined, request: request });
+
+				resolve({ parsedResponse: response, request: request });
 			}, (error: OneNoteApi.RequestError) => {
 				retrieveUserInformationEvent.setStatus(Log.Status.Failed);
 				retrieveUserInformationEvent.setFailureInfo(error);
@@ -172,19 +171,19 @@ export class AuthenticationHelper {
 	/**
 	 * Determines whether or not the given string is valid JSON and has the required elements.
 	 */
-	public isValidUserInformationJsonString(userInfo: string): boolean {
-		let userInfoJson: UserInfoData;
-		try {
-			userInfoJson = JSON.parse(userInfo);
-		} catch (e) {
-			// intentionally not logging this as a JsonParse failure
-			return false;
-		}
-
-		if (userInfoJson && userInfoJson.accessToken && userInfoJson.accessTokenExpiration > 0 && userInfoJson.authType) {
+	protected isValidUserInformation(userInfo: UserInfoData): boolean {
+		if (userInfo && userInfo.accessToken && userInfo.accessTokenExpiration > 0 && userInfo.authType) {
 			return true;
 		}
 
 		return false;
 	}
+
+	/**
+	 * Determines whether or not the given string is valid JSON and has the flag which lets us know if cookies are enabled.
+	 */
+	protected isThirdPartyCookiesEnabled(userInfo: UserInfoData): boolean {
+		// Note that we are returning true by default to ensure the N-1 scenario.
+		return userInfo.cookieInRequest !== undefined ? userInfo.cookieInRequest : true;
+	}
 }

src/scripts/logging/submodules/propertyName.ts

@@ -59,7 +59,8 @@ export module PropertyName {
 		Value,
 		VideoDataOriginalSrcUrl,
 		VideoSrcUrl,
-		Width
+		Width,
+		WriteableCookies
 	}
 
 	/* tslint:disable:variable-name */

src/scripts/userInfo.ts

@@ -8,13 +8,15 @@ export interface UserInfo {
 	lastUpdated?: number;
 	updateReason: UpdateReason;
 	errorDescription?: string;
+	writeableCookies?: boolean;
 }
 
 export interface UserInfoData {
 	accessToken?: string;
 	accessTokenExpiration?: number;
 	authType?: string;
 	cid?: string;
+	cookieInRequest?: boolean;
 	emailAddress?: string;
 	fullName?: string;
 }

src/strings.json

@@ -42,22 +42,25 @@
 	"WebClipper.ClipType.Selection.Button": "Selection",
 	"WebClipper.ClipType.Selection.Button.Tooltip": "Clip the selection you made on the web page.",
 	"WebClipper.ClipType.Selection.ProgressLabel": "Clipping Selection",
-	"WebClipper.Error.NoOpError":  "Sorry, we can\u0027t clip this page right now",
 	"WebClipper.Error.ConflictingExtension": "Your PDF viewer or another extension might be blocking the OneNote Web Clipper. You could temporarily disable the following extension and try clipping again.",
 	"WebClipper.Error.CannotClipPage": "Sorry, this type of page can\u0027t be clipped.",
+	"WebClipper.Error.CookiesDisabled.Line1": "Cookies must be enabled in order for OneNote Web Clipper to work correctly.",
+	"WebClipper.Error.CookiesDisabled.Line2": "Please allow third-party cookies in your browser or add the onenote.com domain as an exception.",
 	"WebClipper.Error.CorruptedSection": "Your clip can\u0027t be saved here because the section is corrupt.",
 	"WebClipper.Error.GenericError": "Something went wrong. Please try clipping the page again.",
+	"WebClipper.Error.GenericExpiredTokenRefreshError": "Your login session has ended and we were unable to clip the page. Please sign in again.",
+	"WebClipper.Error.NoOpError":  "Sorry, we can\u0027t clip this page right now",
 	"WebClipper.Error.NotProvisioned": "Your clip can\u0027t be saved because your OneDrive for Business account isn\u0027t set up.",
+	"WebClipper.Error.OrphanedWebClipperDetected": "Something went wrong. Please refresh this page, and try to clip again.",
 	"WebClipper.Error.PasswordProtected": "Your clip can\u0027t be saved here because the section is password protected.",
 	"WebClipper.Error.QuotaExceeded": "Your clip can\u0027t be saved because your OneDrive account has reached its size limit.",
 	"WebClipper.Error.ResourceDoesNotExist": "Your clip can\u0027t be saved here because the location no longer exists. Please try clipping to another location.",
 	"WebClipper.Error.SectionTooLarge": "Your clip can\u0027t be saved here because the section has reached its size limit.",
+	"WebClipper.Error.SignInUnsuccessful": "We couldn\u0027t sign you in. Please try again.",
+	"WebClipper.Error.ThirdPartyCookiesDisabled": "For OneNote Web Clipper to work correctly, please allow third-party cookies in your browser, or add the onenote.com domain as an exception.",
 	"WebClipper.Error.UserAccountSuspended": "Your clip can\u0027t be saved because your Microsoft account has been suspended.",
 	"WebClipper.Error.UserAccountSuspendedResetText": "Reset Your Account",
 	"WebClipper.Error.UserDoesNotHaveUpdatePermission": "We\u0027ve added features to the Web Clipper that require new permissions. To accept them, please sign out and sign back in.",
-	"WebClipper.Error.GenericExpiredTokenRefreshError": "Your login session has ended and we were unable to clip the page. Please sign in again.",
-	"WebClipper.Error.SignInUnsuccessful": "We couldn\u0027t sign you in. Please try again.",
-	"WebClipper.Error.OrphanedWebClipperDetected": "Something went wrong. Please refresh this page, and try to clip again.",
 	"WebClipper.Extension.RefreshTab": "Please refresh this page, and try to clip again.",
 	"WebClipper.FromCitation": "Clipped from: {0}",
 	"WebClipper.Label.Annotation": "Note",
@@ -128,4 +131,4 @@
 	"WebClipper.FontFamily.Semilight": "Segoe UI Semilight,Segoe UI Light,Segoe WP Light,Segoe UI,Segoe,Segoe WP,Helvetica Neue,Roboto,Helvetica,Arial,Tahoma,Verdana,sans-serif",
 	"WebClipper.FontSize.Preview.SerifDefault": "16px",
 	"WebClipper.FontSize.Preview.SansSerifDefault": "16px"
-}
+}

src/styles/signInPanelStyles.less

@@ -8,10 +8,13 @@
 	padding-bottom: 28px;
 }
 
+.signInErrorCookieInformation {
+	padding-bottom: 10px;
+}
+
 #signInErrorDescription {
 	color: white;
 	font-size: 13px;
-	padding-bottom: 20px;
 	text-align: left;
 	-moz-user-select: text;
 	-khtml-user-select: text;
@@ -20,21 +23,22 @@
 	user-select: text;
 }
 
-.signInErrorDebugInformationContainer {
+#signInErrorDebugInformationContainer {
 	color: white;
 	font-size: 13px;
 	text-align: left;
 }
 
-.signInErrorDebugInformationList {
+#signInErrorDebugInformationList {
 	list-style: none;
 	padding-left: 0;
-	font-size: 13px;
+	font-size: 12px;
 }
 
-.signInErrorToggleInformation {
+#signInErrorToggleInformation {
 	text-align: left;
-	padding-bottom: 10px;
+	padding-bottom: 15px;
+	padding-top: 15px;
 	font-size: 13px;
 }