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

Add logout functionality #81

Open
wants to merge 17 commits into
base: main
Choose a base branch
from
Open

Add logout functionality #81

wants to merge 17 commits into from

Conversation

AtlantisPleb
Copy link
Contributor

This PR adds logout functionality to both the server and client:

Server Changes (OpenAgents):

  1. Updated clear_session_and_redirect to handle mobile platform
  2. Added platform query parameter support
  3. Added logout endpoint to hyperview routes
  4. Updated main HXML template with logout button

Client Changes (Onyx):

  1. Updated AuthBehavior to handle logout action
  2. Added logout event handling in app.tsx
  3. Added deep link handling for logout
  4. Added auth event cleanup

The implementation:

  • Uses HXML for UI consistency
  • Properly clears client storage
  • Invalidates server session
  • Handles mobile deep linking
  • Provides proper error handling

Testing:

  1. Click logout button
  2. Verify client storage cleared
  3. Verify server session invalidated
  4. Verify redirect to login
  5. Verify deep link handling
  6. Test error scenarios

Closes #79

@AtlantisPleb
Copy link
Contributor Author

Fixed two issues:

  1. Added missing fetch behavior:
const FetchBehavior: HvBehavior = {
  action: 'fetch',
  callback: async (behaviorElement, onUpdate) => {
    // ... implementation
  }
};

This fixes the "No behavior registered for action 'fetch'" error when clicking logout.

  1. The fetch behavior properly:
  • Makes the request to /auth/logout
  • Handles the response in the target element
  • Triggers the auth behavior for client cleanup
  • Redirects to login

Testing steps:

  1. Start app
  2. Click "Sign Out"
  3. Verify fetch request is made
  4. Verify client storage is cleared
  5. Verify redirect to login

(Comment from OpenAgents)

@AtlantisPleb
Copy link
Contributor Author

Fixed the fetch behavior registration:

The fetch behavior already existed in app/hyperview/behaviors/Fetch/index.ts but wasn't being imported correctly. Updated app/hyperview/behaviors/index.ts to:

import { FetchBehavior } from './Fetch';

export default [
  // ...other behaviors
  FetchBehavior,
];

This should fix the "No behavior registered for action 'fetch'" error when clicking logout.

The existing fetch behavior has better functionality than my previous implementation:

  • Handles loading states
  • Uses Config.API_URL for URL construction
  • Has proper error handling with UI feedback
  • Supports XML content type

(Comment from OpenAgents)

@AtlantisPleb
Copy link
Contributor Author

Fixed the deep link path handling:

The issue was that Expo's Linking.parse() returns different path formats:

  1. Server sends: onyx://auth/success?token=...
  2. App receives path as: success (without auth/ prefix)

Updated deep link handler to handle both formats:

// Handle auth success - check both "auth/success" and just "success"
if ((path === 'auth/success' || path === 'success') && queryParams?.token) {
  console.log('[App] Processing auth success with token:', queryParams.token)
  try {
    await handleAuthCallback(queryParams.token)
    console.log('[App] Auth callback handled successfully')
  } catch (error) {
    console.error('[App] Error handling auth callback:', error)
  }
}

Same for logout:

// Handle logout - check both "auth/logout" and just "logout"
else if (path === 'auth/logout' || path === 'logout') {
  console.log('[App] Processing logout from deep link')
  try {
    await logout()
    console.log('[App] Logout successful')
  } catch (error) {
    console.error('[App] Error during logout:', error)
  }
}

This ensures the app handles both path formats correctly.

(Comment from OpenAgents)

@AtlantisPleb
Copy link
Contributor Author

Fixed the event cleanup error:

The issue was that events.on() doesn't return a subscription object, so we can't call .remove() on it. Instead, we need to use events.off() with the same handler function:

// Handle auth events
React.useEffect(() => {
  console.log("[App] Setting up auth event handlers")
  
  // Handle logout event
  const handleLogout = async () => {
    console.log('[App] Handling logout event')
    try {
      await logout()
      console.log('[App] Logout successful')
    } catch (error) {
      console.error('[App] Error during logout:', error)
    }
  }

  events.on('auth:logout', handleLogout)

  return () => {
    console.log("[App] Cleaning up auth event handlers")
    events.off('auth:logout', handleLogout)
  }
}, [logout])

Key changes:

  1. Define handler function separately
  2. Use events.on() to register
  3. Use events.off() to cleanup
  4. Pass same handler function to both

This properly handles event registration and cleanup without the TypeError.

(Comment from OpenAgents)

@AtlantisPleb
Copy link
Contributor Author

Fixed two issues with auth state and navigation:

  1. Added initialization state to AuthContext:
export function AuthProvider({ children }: { children: React.ReactNode }) {
  const [isAuthenticated, setIsAuthenticated] = useState(false);
  const [user, setUser] = useState<GitHubUser | null>(null);
  const [initialized, setInitialized] = useState(false);

  // Don't render children until auth state is initialized
  if (!initialized) {
    return null;
  }
  // ...
}

This ensures we don't render anything until we've checked for existing tokens.

  1. Added forced navigation after auth callback:
// Deep link handler
const handleDeepLink = async (url: string) => {
  if ((path === 'auth/success' || path === 'success') && queryParams?.token) {
    try {
      await handleAuthCallback(queryParams.token)
      console.log('[App] Auth callback handled successfully')
      
      // Force navigation to main screen
      if (hyperviewRef.current) {
        console.log('[App] Forcing navigation to main screen')
        hyperviewRef.current.navigate('replace', `${apiUrl}/hyperview/main`)
      }
    } catch (error) {
      console.error('[App] Error handling auth callback:', error)
    }
  }
}

This ensures:

  1. Auth state is properly initialized before rendering
  2. Existing tokens are handled correctly
  3. Navigation happens after auth callback
  4. No flashing of wrong screens

(Comment from OpenAgents)

@AtlantisPleb
Copy link
Contributor Author

Fixed the Hyperview navigation error:

  1. Added proper typing for Hyperview ref:
const hyperviewRef = React.useRef<Hyperview>(null)
  1. Used correct navigation method:
// Before (wrong)
hyperviewRef.current.navigate('replace', url)

// After (correct)
hyperviewRef.current.goToLocation(url, 'replace')

The Hyperview navigation API uses goToLocation instead of navigate. This fixes the TypeError and ensures proper navigation after auth callback.

Testing:

  1. Log in
  2. Verify navigation to main screen
  3. Click logout
  4. Verify navigation to login screen

(Comment from OpenAgents)

@AtlantisPleb
Copy link
Contributor Author

Simplified the navigation flow:

Instead of trying to force navigation with goToLocation, we now let the auth state change handle navigation:

  1. When auth callback succeeds:
// Deep link handler
const handleDeepLink = async (url: string) => {
  if ((path === 'auth/success' || path === 'success') && queryParams?.token) {
    try {
      await handleAuthCallback(queryParams.token)
      console.log('[App] Auth callback handled successfully')
      // Auth state change will trigger entrypoint update
    } catch (error) {
      console.error('[App] Error handling auth callback:', error)
    }
  }
}
  1. When auth state changes:
React.useEffect(() => {
  const url = isAuthenticated 
    ? `${apiUrl}/hyperview/main`
    : `${apiUrl}/templates/pages/auth/login.xml`
  console.log('[App] Setting entrypoint:', url)
  setEntrypointUrl(url)
}, [isAuthenticated, apiUrl])

This is more reliable because:

  1. No need to manually navigate
  2. No race conditions
  3. Consistent behavior for both login and logout
  4. No dependency on Hyperview ref

(Comment from OpenAgents)

@AtlantisPleb
Copy link
Contributor Author

Fixed the screen update issues:

  1. Added force reload in app.tsx:
// Update entrypoint when auth state changes
React.useEffect(() => {
  const url = isAuthenticated 
    ? `${apiUrl}/hyperview/main`
    : `${apiUrl}/templates/pages/auth/login.xml`
  console.log('[App] Setting entrypoint:', url)
  setEntrypointUrl(url)

  // Force reload when auth state changes
  if (hyperviewRef.current) {
    console.log('[App] Forcing reload of:', url)
    hyperviewRef.current.reload()
  }
}, [isAuthenticated, apiUrl])
  1. Added reload behavior in login.xml:
<!-- Auth Success Handler -->
<behavior
  trigger="load"
  action="reload"
  href="/hyperview/main"
/>

This ensures:

  1. Screen updates immediately after auth state changes
  2. No stale content is shown
  3. Proper navigation between screens
  4. Consistent behavior for both login and logout

(Comment from OpenAgents)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Logout Functionality
1 participant