Skip to content

Conversation

@Pbonmars-20031006
Copy link
Contributor

@Pbonmars-20031006 Pbonmars-20031006 commented Dec 17, 2025

Summary

  • update servicenow block to use basic auth instead of oauth, bends the oauth abstraction less

Type of Change

  • Bug fix

Testing

Tested manually

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

@vercel
Copy link

vercel bot commented Dec 17, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Review Updated (UTC)
docs Skipped Skipped Dec 18, 2025 2:32am

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Dec 18, 2025

Greptile Summary

This PR successfully migrates ServiceNow integration from OAuth to basic authentication with username/password credentials.

Key Changes

  • Removed all OAuth-related code (authorization routes, callback handlers, token storage, provider configuration)
  • Added username and password fields to ServiceNow block configuration
  • Created createBasicAuthHeader utility for encoding credentials
  • Updated all four ServiceNow tools (create, read, update, delete) to use basic auth
  • Removed SERVICENOW_CLIENT_ID and SERVICENOW_CLIENT_SECRET environment variables
  • Updated documentation to reflect basic auth instead of OAuth

Architecture Improvements

The migration simplifies the authentication flow significantly by removing the complex OAuth abstraction that didn't fit ServiceNow's multi-instance model well. Basic auth is a cleaner fit since users need to authenticate against their specific ServiceNow instance.

Credentials Handling

The implementation correctly uses visibility: 'user-only' for username and password parameters in all tool configs, following the custom rule 2851870a. Credentials are properly encoded using Base64 and sent via HTTP Authorization headers, which is the standard approach for basic authentication.

The previous OAuth security concerns raised in earlier threads (storing credentials in cookies, passing them via URL parameters) have been completely eliminated since those routes and flows no longer exist.

Confidence Score: 5/5

  • This PR is safe to merge with no identified issues
  • The migration from OAuth to basic auth is cleanly implemented with proper credential handling, follows security best practices with user-only visibility on sensitive fields, removes complex OAuth code that had security concerns, and has passing tests. The basic auth approach is simpler, more appropriate for ServiceNow's architecture, and eliminates the security vulnerabilities present in the previous OAuth implementation.
  • No files require special attention

Important Files Changed

Filename Overview
apps/sim/blocks/blocks/servicenow.ts Completely refactored to use basic auth with username/password fields; removed OAuth-related code
apps/sim/tools/servicenow/utils.ts Added createBasicAuthHeader utility for Base64 encoding credentials
apps/sim/tools/servicenow/create_record.ts Refactored to accept username/password params and use basic auth header
apps/sim/tools/servicenow/read_record.ts Refactored to accept username/password params and use basic auth header
apps/sim/lib/oauth/oauth.ts Removed all ServiceNow OAuth provider configuration and token refresh logic
apps/sim/app/api/auth/servicenow/authorize/route.ts Deleted entire OAuth authorization route - no longer needed with basic auth

Sequence Diagram

sequenceDiagram
    participant User
    participant WorkflowUI as Workflow UI
    participant ServiceNowBlock as ServiceNow Block
    participant Tool as ServiceNow Tool
    participant Utils as Auth Utils
    participant API as ServiceNow API

    User->>WorkflowUI: Configure ServiceNow block
    WorkflowUI->>User: Show username/password fields
    User->>WorkflowUI: Enter instanceUrl, username, password
    
    User->>WorkflowUI: Execute workflow
    WorkflowUI->>ServiceNowBlock: Trigger with params
    ServiceNowBlock->>Tool: Call tool (create/read/update/delete)
    
    Tool->>Tool: Validate instanceUrl, username, password
    Tool->>Utils: createBasicAuthHeader(username, password)
    Utils->>Utils: Base64 encode credentials
    Utils->>Tool: Return "Basic {encoded}"
    
    Tool->>API: HTTP request with Authorization header
    API->>API: Validate credentials
    API->>Tool: Return response
    
    Tool->>ServiceNowBlock: Return formatted result
    ServiceNowBlock->>WorkflowUI: Return output
    WorkflowUI->>User: Display result
Loading

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

Additional Comments (1)

  1. apps/sim/app/api/auth/servicenow/authorize/route.ts, line 179-192 (link)

    logic: added validation for credentials in JavaScript, but validation happens after credentials are already exposed in the URL - they're already logged by this point

    move to POST endpoint to prevent credential exposure in URLs and logs

9 files reviewed, 8 comments

Edit Code Review Agent Settings | Greptile

@waleedlatif1 waleedlatif1 force-pushed the feat/servicenow-update branch from e6096c4 to 68bb7ac Compare December 18, 2025 02:20
@waleedlatif1 waleedlatif1 changed the title fix(ServiceNow-creds): Fixing ServiceNow credential issue by adding client ID and secret fields fix(servicenow): update servicenow block to use basic auth instead of oatuh Dec 18, 2025
@waleedlatif1 waleedlatif1 changed the title fix(servicenow): update servicenow block to use basic auth instead of oatuh fix(servicenow): update servicenow block to use basic auth instead of oauth Dec 18, 2025
@waleedlatif1
Copy link
Collaborator

@greptile

@waleedlatif1 waleedlatif1 merged commit 491bd78 into staging Dec 18, 2025
11 checks passed
@waleedlatif1 waleedlatif1 deleted the feat/servicenow-update branch December 18, 2025 04:41
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.

4 participants